Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#264] Zero pad the hexadecimal pid in the backup temporary file name to keep its length at 4-bytes even if the process_id is less than 2**24 #269

Merged
merged 2 commits into from
Jun 1, 2018

Conversation

nars1
Copy link
Collaborator

@nars1 nars1 commented Jun 1, 2018

No description provided.

…le name to keep its length at 4-bytes even if the process_id is less than 2**24
@nars1 nars1 self-assigned this Jun 1, 2018
@nars1 nars1 requested a review from estess June 1, 2018 13:52
* is a small number (< 2**24 etc.) and is 0-filled on the left if needed. Makes the length
* deterministic instead of being dependent on the process_id.
*/
nbytes = SNPRINTF(tempfilename, SIZEOF(tempfilename), "%.*s/%.*s_%04x_XXXXXX",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter that we might be truncating a pid here? PIDs are usually < 32K but the pid_t itself is an int and I thought there was a configuration option on the Linux kernel to widen the used PID field once storage passed a certain point or some such thing - can't remember. Anyway, should this be 8x instead of 4x to properly represent a 32 bit value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I forgot that hex digits are twice the byte length so it should be 8x instead of 4x. Will take care of that.

@ksbhaskar
Copy link
Member

ksbhaskar commented Jun 1, 2018 via email

@nars1 nars1 merged commit 73c68d0 into YottaDB:master Jun 1, 2018
@nars1 nars1 deleted the backup branch June 1, 2018 17:37
@nars1
Copy link
Collaborator Author

nars1 commented Jun 1, 2018

For the record. Even though this pull request identifies the issue # as #264, it actually fixes #267.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants