mbox series

[0/4] btrfs-progs: Compiling warning fixes for devel branch

Message ID 20191118063052.56970-1-wqu@suse.com (mailing list archive)
Headers show
Series btrfs-progs: Compiling warning fixes for devel branch | expand

Message

Qu Wenruo Nov. 18, 2019, 6:30 a.m. UTC
We have several compiling errors, in devel branch.
One looks like a false alert from compiler, the first patch will
workaround it.

3 warning from libbtrfsutils are due to python3.8 changes.
Handle it properly by using designated initialization, which also saves
us quite some lines.

Qu Wenruo (4):
  btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
  btrfs-progs: libbtrfsutil: Convert to designated initialization for
    BtrfsUtilError_type
  btrfs-progs: libbtrfsutil: Convert to designated initialization for
    QgroupInherit_type
  btrfs-progs: libbtrfsutil: Convert to designated initialization for
    SubvolumeIterator_type

 check/mode-common.c             |  2 +-
 libbtrfsutil/python/error.c     | 49 ++++++++-------------------------
 libbtrfsutil/python/qgroup.c    | 43 ++++++-----------------------
 libbtrfsutil/python/subvolume.c | 44 ++++++-----------------------
 4 files changed, 30 insertions(+), 108 deletions(-)

Comments

Nikolay Borisov Nov. 18, 2019, 8:23 a.m. UTC | #1
On 18.11.19 г. 8:30 ч., Qu Wenruo wrote:
> We have several compiling errors, in devel branch.
> One looks like a false alert from compiler, the first patch will
> workaround it.
> 
> 3 warning from libbtrfsutils are due to python3.8 changes.
> Handle it properly by using designated initialization, which also saves
> us quite some lines.
> 
> Qu Wenruo (4):
>   btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     BtrfsUtilError_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     QgroupInherit_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     SubvolumeIterator_type
> 
>  check/mode-common.c             |  2 +-
>  libbtrfsutil/python/error.c     | 49 ++++++++-------------------------
>  libbtrfsutil/python/qgroup.c    | 43 ++++++-----------------------
>  libbtrfsutil/python/subvolume.c | 44 ++++++-----------------------
>  4 files changed, 30 insertions(+), 108 deletions(-)
> 

For the whole series:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
David Sterba Nov. 18, 2019, 6:10 p.m. UTC | #2
On Mon, Nov 18, 2019 at 02:30:48PM +0800, Qu Wenruo wrote:
> We have several compiling errors, in devel branch.
> One looks like a false alert from compiler, the first patch will
> workaround it.
> 
> 3 warning from libbtrfsutils are due to python3.8 changes.
> Handle it properly by using designated initialization, which also saves
> us quite some lines.
> 
> Qu Wenruo (4):
>   btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     BtrfsUtilError_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     QgroupInherit_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     SubvolumeIterator_type

Added to devel, thanks. Please note that libbtrfsutil does not need the
btrfs-progs prefix, as it's considered a separate part.
Omar Sandoval Nov. 18, 2019, 6:27 p.m. UTC | #3
On Mon, Nov 18, 2019 at 02:30:48PM +0800, Qu Wenruo wrote:
> We have several compiling errors, in devel branch.
> One looks like a false alert from compiler, the first patch will
> workaround it.
> 
> 3 warning from libbtrfsutils are due to python3.8 changes.
> Handle it properly by using designated initialization, which also saves
> us quite some lines.
> 
> Qu Wenruo (4):
>   btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     BtrfsUtilError_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     QgroupInherit_type
>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>     SubvolumeIterator_type
> 
>  check/mode-common.c             |  2 +-
>  libbtrfsutil/python/error.c     | 49 ++++++++-------------------------
>  libbtrfsutil/python/qgroup.c    | 43 ++++++-----------------------
>  libbtrfsutil/python/subvolume.c | 44 ++++++-----------------------
>  4 files changed, 30 insertions(+), 108 deletions(-)

Thanks for fixing the libbtrfsutil parts. For some reason, the
convention for Python C extensions is to not use designated
initializers, but after this breakage it's definitely safer to use them.

I guess Dave already merged these, but FWIW,

Reviewed-by: Omar Sandoval <osandov@fb.com>
Qu Wenruo Nov. 18, 2019, 11:47 p.m. UTC | #4
On 2019/11/19 上午2:27, Omar Sandoval wrote:
> On Mon, Nov 18, 2019 at 02:30:48PM +0800, Qu Wenruo wrote:
>> We have several compiling errors, in devel branch.
>> One looks like a false alert from compiler, the first patch will
>> workaround it.
>>
>> 3 warning from libbtrfsutils are due to python3.8 changes.
>> Handle it properly by using designated initialization, which also saves
>> us quite some lines.
>>
>> Qu Wenruo (4):
>>   btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
>>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>>     BtrfsUtilError_type
>>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>>     QgroupInherit_type
>>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
>>     SubvolumeIterator_type
>>
>>  check/mode-common.c             |  2 +-
>>  libbtrfsutil/python/error.c     | 49 ++++++++-------------------------
>>  libbtrfsutil/python/qgroup.c    | 43 ++++++-----------------------
>>  libbtrfsutil/python/subvolume.c | 44 ++++++-----------------------
>>  4 files changed, 30 insertions(+), 108 deletions(-)
> 
> Thanks for fixing the libbtrfsutil parts. For some reason, the
> convention for Python C extensions is to not use designated
> initializers, but after this breakage it's definitely safer to use them.
> 
> I guess Dave already merged these, but FWIW,
> 
> Reviewed-by: Omar Sandoval <osandov@fb.com>
> 
A small question inspired by this bug, but not specific to btrfs.

Does this mean each big python upgrade makes all c-binding binary
incompatible? Or this is just a hiccup for this python3.8 release?

If it's the former case, that doesn't look correct to me...

Thanks,
Qu
Omar Sandoval Nov. 19, 2019, 8:38 p.m. UTC | #5
On Mon, Nov 18, 2019 at 11:47:41PM +0000, Qu WenRuo wrote:
> 
> 
> On 2019/11/19 上午2:27, Omar Sandoval wrote:
> > On Mon, Nov 18, 2019 at 02:30:48PM +0800, Qu Wenruo wrote:
> >> We have several compiling errors, in devel branch.
> >> One looks like a false alert from compiler, the first patch will
> >> workaround it.
> >>
> >> 3 warning from libbtrfsutils are due to python3.8 changes.
> >> Handle it properly by using designated initialization, which also saves
> >> us quite some lines.
> >>
> >> Qu Wenruo (4):
> >>   btrfs-progs: check/lowmem: Fix a false alert on uninitialized value
> >>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
> >>     BtrfsUtilError_type
> >>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
> >>     QgroupInherit_type
> >>   btrfs-progs: libbtrfsutil: Convert to designated initialization for
> >>     SubvolumeIterator_type
> >>
> >>  check/mode-common.c             |  2 +-
> >>  libbtrfsutil/python/error.c     | 49 ++++++++-------------------------
> >>  libbtrfsutil/python/qgroup.c    | 43 ++++++-----------------------
> >>  libbtrfsutil/python/subvolume.c | 44 ++++++-----------------------
> >>  4 files changed, 30 insertions(+), 108 deletions(-)
> > 
> > Thanks for fixing the libbtrfsutil parts. For some reason, the
> > convention for Python C extensions is to not use designated
> > initializers, but after this breakage it's definitely safer to use them.
> > 
> > I guess Dave already merged these, but FWIW,
> > 
> > Reviewed-by: Omar Sandoval <osandov@fb.com>
> > 
> A small question inspired by this bug, but not specific to btrfs.
> 
> Does this mean each big python upgrade makes all c-binding binary
> incompatible? Or this is just a hiccup for this python3.8 release?
> 
> If it's the former case, that doesn't look correct to me...

In general, each major Python version may change the C extension ABI,
which is why the Python version is encoded in the file name:

/usr/lib/python3.8/site-packages/btrfsutil.cpython-38-x86_64-linux-gnu.so

Usually they maintain source-level compatibility, but it appears that
CPython itself uses 0 for its unused initializers rather than NULL, so
they probably just missed this.