Free upon errors in config file(s) #133
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm going to work on a simpler/cleaner way of doing this. |
gperciva
force-pushed
the
free-config-error
branch
from
a52cd2e
to
5130e81
Compare
|
This now consists of the first 2 commits of the original PR. It leaves a FILE open and a 8192-char buffer, both from |
|
|
||
| /* Find the start of the argument. */ | ||
| conf_arg = conf_opt + optlen + 1; | ||
| conf_arg += strspn(conf_arg, " \t"); | ||
| bsdtar->conf_arg = bsdtar->conf_opt + optlen + 1; |
There was a problem hiding this comment.
You're setting bsdtar->conf_arg to a string which was not allocated by malloc. What happens if we exit from this point?
gperciva
force-pushed
the
free-config-error
branch
from
5130e81
to
1915b90
Compare
|
Updated PR. |
| } else { | ||
| conf_arg_malloced = NULL; | ||
| bsdtar->conf_arg = NULL; |
This moves the "allocated" version of conf_opt into the "global" bsdtar struct because if there's a config file error, we'll exit inside dooption(), leaving the previous local variable conf_opt un-freed.
This moves conf_arg_malloced into the "global" bsdtar struct because if there's a config file error, we'll exit inside dooption(), leaving the previous local variable conf_arg_malloced un-freed.
gperciva
force-pushed
the
free-config-error
branch
from
1915b90
to
5be74bc
Compare
|
Updated, and should be easier to read / track history now. |
|
Looks good to me. Should I merge now or did you want to rebase anything? |
|
Go ahead and merge it now. More granularity in the history isn't a bad thing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes valgrind happy if there's any problems in parsing the config file. I'm not 100% happy with this PR since it's a bit awkward due to interfacing with libarchive code (longer comments in a52cd2e), but it's worth bringing to your attention.