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

Adjust printf specifiers in examples code #5146

Merged
merged 1 commit into from
Aug 2, 2019
Merged

Adjust printf specifiers in examples code #5146

merged 1 commit into from
Aug 2, 2019

Conversation

scottfurry
Copy link
Contributor

Static analysis of example code found multiple findings of printf usage
where filling value is members of git_indexer_progress object. Specifier
used was for signed int but git_indexer_progress members are typed as
unsigned int. printf specifiers were altered to match type.

@pks-t
Copy link
Member

pks-t commented Jul 19, 2019

Could you please rebase your PR to fix conflicts? There's been some changes to the examples code due to #5143, so it's possible that your fix has already been included via it

@scottfurry
Copy link
Contributor Author

PR rebased to latest master branch. There was one conflict in examples/clone.c that had to be resolved but the rest were as before.

Copy link
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

Sorry that it took me so long to review :/ One conversion is unfortunately wrong and fails the build, but as soon as you've fixed up that one I'm happy to merge!

examples/clone.c Outdated
pd->fetch_progress.indexed_deltas,
pd->fetch_progress.total_deltas);
} else {
printf("net %3d%% (%4"PRIuZ" kb, %5d/%5d) / idx %3d%% (%5d/%5d) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ") %s\n",
printf("net %3d%% (%4d kb, %5u/%5u) / idx %3d%% (%5u/%5u) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ") %s\n",
Copy link
Member

Choose a reason for hiding this comment

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

The %u conversions are the right thing to do, but the "PRIuZ" needs to stay as kbytes is of type size_t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that I missed that when I reworked original after a source change. Will fix this residual.

examples/clone.c Outdated
pd->fetch_progress.indexed_deltas,
pd->fetch_progress.total_deltas);
} else {
printf("net %3d%% (%4"PRIuZ" kb, %5d/%5d) / idx %3d%% (%5d/%5d) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ") %s\n",
printf("net %3d%% (%4"PRIuz" kb, %5u/%5u) / idx %3d%% (%5u/%5u) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ")
Copy link
Member

Choose a reason for hiding this comment

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

It's PRIuZ, not PRIuz ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My error in not paying attention to detail.

Copy link
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

My error in not paying attention to detail.

Well, errors happen, we're all just human. That's why I'd recommend to compile the code previous to committing to keep your own frustration level low :) Immediate feedback is a hugely valuable thing!

examples/clone.c Outdated
printf("net %3d%% (%4"PRIuZ" kb, %5d/%5d) / idx %3d%% (%5d/%5d) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ") %s\n",
printf("net %3d%% (%4" PRIuZ " kb, %5u/%5u) / idx %3d%% (%5u/%5u) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ
")
%s\n",
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you've now introduced some line breaks by accident that break compilation 🙄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I did the change in the terminal(nano) and word wrap kicked in.

Static analysis of example code found multiple findings of `printf` usage
where filling value is members of git_indexer_progress object. Specifier
used was for signed int but git_indexer_progress members are typed as
unsigned ints. `printf` specifiers were altered to match type.
Copy link
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

Now we're good to go! :) Thanks a lot, @scottfurry

@pks-t pks-t merged commit 24c491e into libgit2:master Aug 2, 2019
@scottfurry
Copy link
Contributor Author

Touchdown - crowd goes wild!!!!

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.

2 participants