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 _______________________________________________ netcf-devel mailing list netcf-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/netcf-devel