Re: [Qemu-devel] [PATCH v3] qemu-io: Add generic function for reinitializing optind.

[Daniel P . Berrangé]

On Wed, Jan 09, 2019 at 02:14:46PM +0000, Richard W.M. Jones wrote:
> On Wed, Jan 09, 2019 at 01:44:48PM +0000, Daniel P. Berrangé wrote:
> > On Wed, Jan 09, 2019 at 01:23:01PM +0000, Richard W.M. Jones wrote:
> > > How about this one?  Add a generic osdep function for reinitializing
> > > optind, which does optreset on FreeBSD (but is identical on all other
> > > OSes).  Use it from qemu-io and qemu-img.
> > > 
> > > I have tested this on Linux, FreeBSD and OpenBSD.
> > > 
> > > checkpatch complains:
> > > 
> > > WARNING: Block comments use a leading /* on a separate line
> > > #69: FILE: include/qemu/osdep.h:591:
> > > +/**
> > 
> > I think it just doesn't like your '/**' and wants '/*' instead.
> 
> The existing comments in the same file are a mix of three styles.
> I chose the style used closest to the new comment I was adding :-)

Yeah, pre-existing code is a mess often not passing current style
rules. I wish we'd clean up existing code, but failing that, it can
be valid to ignore style warnings to be more consistent with existing
code.

> 
> > > WARNING: architecture specific defines should be avoided
> > > #78: FILE: include/qemu/osdep.h:600:
> > > +#ifdef __FreeBSD__
> > 
> > Normally we'd suggest doing a configure test to for the platform
> > feature and then using a feature based ifdef test. In this case
> > though that would be difficult and/or overly complex.
> > 
> > This does make me wonder about the other *BSDs, OS-X and Mingw
> 
> OpenBSD is known fine with optind = 0.  I can't test OS-X.  I can test
> mingw (on Linux) later.
> 
> 
> > though ?  Should they all be using the #else codepath, or should
> > the other BSDs / OS-X use the __FreeBSD__ codepath.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|