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

Various fixes based on coverity errors #405

merged 51 commits into from Oct 4, 2016


Copy link

@lutter lutter commented Sep 29, 2016

This fixes are all based on running augeas through coverity.

The main reason to make these macros is that it makes sure we set
state->regs to NULL when we save registers so that any error path does not
inadvertently free the caller's registers.

Before we inconsistently set state->regs to NULL manually, and there were
cases where we could have freed the caller's registers when encountering
certain errors.
We were calling free on a pointer into the interior of an array
When we ran out of memory, we did not properly free the previously
allocated LOCPATH.
This allows us to remove some confused checking
These routines are now more resilient to being passed NULL arguments
The TREE argument must not be NULL, so there's no need to check for that
later on
Fixing this to make coverity happy - it's unused code in a debugging routine
If we succeed in allocating INI ourselves, but then fail to push the
initial state into it, we would leak INI. Fix that by explicitly freeing
When we tried to allocate BODY and allocation failed, we still had
allocated memory in PARAMS that was not freed in the error label. We now
free that list on error.

To make sure we do not try and free PARAMS when it has been linked into
FUNC, we set it to NULL after the call to BUILD_FUNC.

The call to BUILD_FUNC also erroneously checked the return value for NULL,
even though this function can never return NULL. This check was removed.
We used to call STRDUP for no good reason on the constant error message
returned by PATHX_ERROR and promptly leaked it.

make_pathx_exn already increases the reference count appropriately. The
additional reference caused INFO to be leaked.
When parse failed, and we needed to skip the actual put, we used to return
straight away, even though we were holding on to a number of intermediate
data structures. Now we properly clean them up.
Otherwise, we might try to free cmd.opt before it has been set to NULL.
If we encountered an (internal) error in ENSURE, it was possible that we
leaked SKEL and DICT. We now make sure they get set to NULL when we have
saved them, and free them if they are not NULL yet.
This avoids false positives with address sanitizer
Copy link
Member Author

lutter commented Oct 2, 2016

With the latest updates to this PR, pretty much all coverity errors have been addressed, and the build passes for me locally with ASAN turned on.

When we went to clean up files that are not managed by a lens anymore, we
just assumed that anything with a 'path' child held metadata for a file and
removed that metadata and the actual contents.

That could lead to a situation where creating a node /augeas/files/path
would lead to a crash. We now use an explicit flag in the tree node that
holds file metadata to indicate the fact rather than just inferring it from
the presence of a 'path' child, so that we will never inadvertently free
everything under /augeas/files.
This file used tabs for indentation which causes errors about misleading
indentation with GCC 6. Replacing tabs with spaces fixes that.
@lutter lutter merged commit 0d8c7bb into hercules-team:master Oct 4, 2016
@lutter lutter deleted the dev/cov branch October 4, 2016 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

1 participant