Skip to content

A few more fixes from coverity #3201

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

Merged
merged 4 commits into from
Jun 10, 2015
Merged

A few more fixes from coverity #3201

merged 4 commits into from
Jun 10, 2015

Conversation

carlosmn
Copy link
Member

This is what I managed to get done this morning. There's some very real issues fixed in this branch plus a bit of readability.

Unfortunately I haven't managed to figure out how to tell it to ignore memory leaks in case of OOM so it's not the nicest thing to drudge through the issues to find the real ones.

carlosmn added 4 commits June 10, 2015 10:30
`merge_diff_list_count_candidates()` takes pointers to the source and
target counts, but when it comes time to increase them, we're increasing
the pointer, rather than the value it's pointing to.

Dereference the value to increase.
The way we currently do it depends on the subtlety of strlen vs sizeof
and the fact that .pack is one longer than .idx. Let's use a git_buf so
we can express the manipulation we want much more clearly.
We take in a possibly partial ID by taking a length and working off of
that to figure out whether to just look up the object or ask the
backends for a prefix lookup.

Unfortunately we've been checking the size against `GIT_OID_HEXSZ` which
is the size of a *string* containing a full ID, whereas we need to check
against the size we can have when it's a 20-byte array.

Change the checks and comment to use `GIT_OID_RAWSZ` which is the
correct size of a git_oid to have when full.
When we hit an error writing to the next stream from a file, we jump to
'done' which currently skips over closing the file descriptor.

Make sure to close the descriptor if it has been set to a valid value.
@carlosmn carlosmn changed the title A few more fixes from coverit A few more fixes from coverity Jun 10, 2015
ethomson added a commit that referenced this pull request Jun 10, 2015
A few more fixes from coverity
@ethomson ethomson merged commit da6720f into master Jun 10, 2015
@carlosmn carlosmn deleted the cmn/coverity branch June 23, 2015 14:11
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