[Libguestfs] [PATCH nbdkit] server: Use bool for types which are really booleans.

[Richard W.M. Jones]

On Wed, Jan 02, 2019 at 08:54:15AM -0600, Eric Blake wrote:
> On 1/1/19 12:21 PM, Richard W.M. Jones wrote:
> > For mainly historical reasons we tended to use int to store boolean
> > values.  However using bool is probably safer in some corner cases
> > (eg. ‘v == true’ can fail badly if v is an int, but works for bool).
> 
> The problems only occur when v is an int and set to something other than
> 0 or 1.  But yes, in general using the bool type is worth doing.
> 
> > bool was added in C99 so let's use it.
> > ---
> >  server/internal.h    | 16 +++++------
> >  server/connections.c | 28 +++++++++---------
> >  server/crypto.c      |  4 +--
> >  server/main.c        | 67 ++++++++++++++++++++++----------------------
> >  server/plugins.c     |  2 +-
> >  5 files changed, 59 insertions(+), 58 deletions(-)
> 
> 
> > +++ b/server/connections.c
> > @@ -78,13 +78,13 @@ struct connection {
> >    uint32_t cflags;
> >    uint64_t exportsize;
> >    uint16_t eflags;
> > -  int readonly;
> > -  int can_flush;
> > -  int is_rotational;
> > -  int can_trim;
> > -  int can_zero;
> > -  int can_fua;
> > -  int using_tls;
> > +  bool readonly;
> > +  bool can_flush;
> > +  bool is_rotational;
> > +  bool can_trim;
> > +  bool can_zero;
> > +  bool can_fua;
> > +  bool using_tls;
> 
> Some of these were 'int' because they have tri-state returns from the
> client (-1 for error, or 0/1 for success).  I suppose that making them
> bool means that you only store into it after checking for errors, but it
> does mean that we have to audit a bit more carefully that we aren't
> accidentally turning -1 into true.

Yes in the public API, but as far as I can see in this struct they are
all really booleans, ie. all of the code is like:

  fl = backend->can_flush (backend, conn);
  if (fl == -1)
    return -1;
  if (fl) {
    eflags |= NBD_FLAG_SEND_FLUSH;
    conn->can_flush = 1;
  }

  ...

  if (!conn->flush ...

> But I didn't spot any obvious problems, so I think the patch is good to go.

So yes it's my belief also that this change is safe.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top