On Thu, Feb 07, 2019 at 07:12:08AM -0500, John Ferlan wrote:
>
>
> On 2/7/19 4:10 AM, Erik Skultety wrote:
> > On Wed, Feb 06, 2019 at 08:41:33AM -0500, John Ferlan wrote:
> >> Let's make use of the auto __cleanup capabilities cleaning up any
> >> now unnecessary goto paths. For virStorageAuthDefCopy use authdef
> >> and ret as consistently as similar other code.
> >>
> >> Signed-off-by: John Ferlan <jfer...@redhat.com>
> >> ---
> >> src/conf/domain_conf.c | 29 +++++++++++------------------
> >> src/conf/storage_conf.c | 6 ++----
> >> src/qemu/qemu_parse_command.c | 6 ++----
> >> src/util/virstoragefile.c | 33 ++++++++++++++-------------------
> >> src/util/virstoragefile.h | 1 +
> >> 5 files changed, 30 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >> index 1fc4c8a35a..5699a60549 100644
> >> --- a/src/conf/domain_conf.c
> >> +++ b/src/conf/domain_conf.c
> >> @@ -7578,10 +7578,9 @@
> >> virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
> >> virDomainHostdevSubsysSCSIPtr
> >> def,
> >> xmlXPathContextPtr ctxt)
> >> {
> >> - int ret = -1;
> >> int auth_secret_usage = -1;
> >> xmlNodePtr cur;
> >> - virStorageAuthDefPtr authdef = NULL;
> >> + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL;
> >
> > For better readability I prefer declaring VIR_AUTO variables at the end of
> > the
> > declare block (multiple occurrences throughout the patch)...
> >
> > ...
> >
>
> I don't mind moving them. I generally just try to keep the usages together.
>
>
> >> + VIR_STEAL_PTR(iscsisrc->src->auth, authdef);
> >
> > ^Unrelated change, there should be a trivial patch preceding this one taking
> > care of the VIR_STEAL_PTR replacements.
> >
> > ...
>
> You'll find this sprinkled throughout - generating separate patches
> could explode the series into perhaps 30+ patches which I doubt anyone
> would jump at the chance to review. I can separate before pushing and I
> assume by extracting it/them I can just add your R-by too...
That's why I mentioned trivial, I don't need to review those, as long as you
split them before pushing, I'm fine with that and the Rb applies, sorry for not
being clearer about that.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
