Re: [libvirt] [PATCH v3 02/11] virt-admin: Introduce first working skeleton
> + * You should have received a copy of the GNU Lesser General
Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>;.
> + *
> + * Erik Skultety <eskultet(a)redhat.com>
We usually prepend this line with "Authors: ".
How can I keep forgetting these things?! I'll fix it, thanks.
> +static bool
> +cmdConnect(vshControl *ctl, const vshCmd *cmd)
> +{
> + const char *name = NULL;
> + vshAdmControlPtr priv = ctl->privData;
> +
> + VIR_FREE(ctl->connname);
> + if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
> + return false;
I think the VIR_FREE() should go after option acquiring. Otherwise we
will lose it if vshCommand... fails.
Spurious as it might be, this could only fail, if we decided that --name
argument to virsh/virt-admin connect command should be required, but
since it's optional, vshCommandOptStringReq can never fail in this
specific case (I guess I already mentioned this in one of my earlier
replies to previous versions). But I do agree with you that it would be
nice to fix the logic, so no future questions about this snippet of code
will arise.
> + ctl->connname = vshStrdup(ctl, name);
> +
> + vshAdmReconnect(ctl);
> +
> + return !!priv->conn;
> +}
ACK
Michal
Thanks, I'll adjust the patch and push it once the series is complete.
Erik