Re: [netcf-devel] [PATCH v2] Report file path and reason when aug_save fails
On 08/09/14 16:51, Laine Stump wrote:
On 09/05/2014 04:21 AM, Dominic Cleal wrote:
> When aug_save fails, the /augeas metadata tree is now searched to find a
> possible reason for the failure and an error is raised containing the file
> path, Augeas error code (pointing to the action that failed) and the message if
> supplied (from strerror).
>
> A failure to unlink a file now results in a message such as:
>
> error: unspecified error
> error: aug_save failed on /etc/sysconfig/network-scripts/ifcfg-em1:
> unlink_orig (Permission denied)
>
> Multiple failures aren't reported in detail, but will be printed if debug is
> enabled.
Thanks for taking the time to do this! (and you even tested building on
debian and suse, which is greatly appreciated since I don't have systems
setup to do that).
The one change I made was to turn this:
> + if (aug_save_assert(ncf) < 0)
> + goto error;
into this:
aug_save_assert(ncf);
ERR_BAIL(ncf);
just for consistency with the rest of the code (I didn't create the
ERR_*() macros, and have never been sure if I really liked them or not,
but consistency is nice, and that's the way everything in the netcf code
is written, so I've tried to keep it that way).
Thanks for the fixes, that makes a lot more sense.
Unsurprisingly, the style is very similar to core Augeas!
I *didn't* change the following to use ERR_COND_BAIL() though,
because I
think the logic that is made obvious by the if() ... else ... constructs
would be obscured too much by the goto's that are implicit in
ERR_COND_BAIL():
Agreed.
Cheers,
--
Dominic Cleal
Red Hat Engineering