Libguestfs - Jul 2019 - Re: [PATCH libnbd v2 5/5] lib: Use unsigned for pread_structured status parameter.

On 7/24/19 11:54 AM, Richard W.M. Jones wrote:
> This is a bitmask so using an unsigned type is slightly safer.  This
> is not an ABI change since the types are compatible.
> 
> Thanks: Eric Blake.
> ---
>  examples/strict-structured-reads.c |  2 +-
>  generator/generator                | 67 ++++++++++++++++--------------
>  interop/structured-read.c          |  2 +-
>  lib/internal.h                     |  2 +-
>  tests/oldstyle.c                   |  2 +-
>  5 files changed, 39 insertions(+), 36 deletions(-)
> 
> @@ -3789,8 +3789,8 @@ let print_python_binding name { args; ret } >     
pr "    for (size_t i = 0; i < %s; ++i)\n" len;
>              pr "      PyList_SET_ITEM (py_%s, i,
PyLong_FromUnsignedLong (%s[i]));\n" n n
>           | BytesIn _
> -           | Int _
> -           | Int64 _ -> ()
> +         | Int _
> +         | Int64 _ -> ()
A bit of churn here that could have been squashed into patch 2.  But at
the end of the day, whether to spend more time polishing the patch when
the end result is fine is up to you.

ACK series. I'll rebase my auto-retire patches on top of this once
you've pushed.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Wed, Jul 24, 2019 at 01:43:23PM -0500, Eric Blake
wrote:
> On 7/24/19 11:54 AM, Richard W.M. Jones wrote:
> > This is a bitmask so using an unsigned type is slightly safer.  This
> > is not an ABI change since the types are compatible.
> > 
> > Thanks: Eric Blake.
> > ---
> >  examples/strict-structured-reads.c |  2 +-
> >  generator/generator                | 67
++++++++++++++++--------------
> >  interop/structured-read.c          |  2 +-
> >  lib/internal.h                     |  2 +-
> >  tests/oldstyle.c                   |  2 +-
> >  5 files changed, 39 insertions(+), 36 deletions(-)
> > 
> 
> > @@ -3789,8 +3789,8 @@ let print_python_binding name { args; ret } >
>              pr "    for (size_t i = 0; i < %s; ++i)\n" len;
> >              pr "      PyList_SET_ITEM (py_%s, i,
PyLong_FromUnsignedLong (%s[i]));\n" n n
> >           | BytesIn _
> > -           | Int _
> > -           | Int64 _ -> ()
> > +         | Int _
> > +         | Int64 _ -> ()
> 
> A bit of churn here that could have been squashed into patch 2.  But at
> the end of the day, whether to spend more time polishing the patch when
> the end result is fine is up to you.
> 
> ACK series. I'll rebase my auto-retire patches on top of this once
> you've pushed.
I think I need a bit more time on this series.  How about you
push yours and I'll rebase this one on top of it.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On 7/24/19 2:03 PM, Richard W.M. Jones wrote:
> On Wed, Jul 24, 2019 at 01:43:23PM -0500, Eric Blake wrote:
>> On 7/24/19 11:54 AM, Richard W.M. Jones wrote:
>>> This is a bitmask so using an unsigned type is slightly safer. 
This
>>> is not an ABI change since the types are compatible.
>>>
>> A bit of churn here that could have been squashed into patch 2.  But at
>> the end of the day, whether to spend more time polishing the patch when
>> the end result is fine is up to you.
>>
>> ACK series. I'll rebase my auto-retire patches on top of this once
>> you've pushed.
> 
> I think I need a bit more time on this series.  How about you
> push yours and I'll rebase this one on top of it.
Okay, mine is in.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org