All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Pali Rohár" <pali@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Jan Kara" <jack@suse.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Steven J. Magnani" <steve@digidescorp.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH 09/15] udf: avoid gcc-10 zero-length-bounds warnings
Date: Fri, 1 May 2020 22:48:43 +0200	[thread overview]
Message-ID: <20200501204843.GA9544@quack2.suse.cz> (raw)
In-Reply-To: <CAK8P3a0_BuKDK7X8dTHyx7t++CGjy3jhDVvL=Jo5=kcUxmtobg@mail.gmail.com>

On Fri 01-05-20 22:30:27, Arnd Bergmann wrote:
> On Thu, Apr 30, 2020 at 11:54 PM Pali Rohár <pali@kernel.org> wrote:
> > > @@ -360,9 +360,9 @@ struct logicalVolIntegrityDesc {
> > >       uint8_t                 logicalVolContentsUse[32];
> > >       __le32                  numOfPartitions;
> > >       __le32                  lengthOfImpUse;
> > > -     __le32                  freeSpaceTable[0];
> > >       __le32                  sizeTable[0];
> > >       uint8_t                 impUse[0];
> > > +     __le32                  freeSpaceTable[];
> >
> > Please do not change order of members in these structures. Order is
> > strictly defined by ECMA 167 standard and changing them you would just
> > confuse reader. In LVID is free space table before size table.
> 
> Ok
> 
> > If you do not like GNU C extension for zero-length arrays then just
> > replace it by standard C99 flexible arrays. I think that there is no
> > reason to not use standard C99 language constructions, just nobody had
> > motivation or time to change (working) code.
> 
> No, the problem is that only the last member can be a flexible array,
> so when impUse[] is the last member, freeSpaceTable has to be a zero
> length array.
> 
> []> Also this file is semi-synchronized with udftools project in which I
> > already replaced all GNU C zero-length arrays by C99 flexible arrays.
> >
> > You can take inspiration what I did with logicalVolIntegrityDesc:
> > https://github.com/pali/udftools/commit/f851d84478ce881d516a76018745fa163f803880#diff-1e1a5b89f620d380f22b973f9449aeaeL381-R384
> 
> Right, this is likely the best workaround.
> 
> > Anyway, if you have a better idea what to do with such on-disk structure
> > and how to represent it in C struct syntax, let me know as it could be
> > updated also in udftools project.
> 
> The trick I used for impUse[] would also work for freeSpaceTable[] to avoid
> the gcc warning, it's still not great, but maybe you like this better:

I like Pali's version somewhat better because whenever I look at several
(obviously flexible) arrays in one struct, I start wondering what's going
on. So let's not define members of struct whose offset we actually don't
know (and thus we cannot sanely use them anyway).

								Honza
 
> arnd@threadripper:~/arm-soc$ git diff
> diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
> index 02f03fadb75b..666d022eb00b 100644
> --- a/fs/udf/balloc.c
> +++ b/fs/udf/balloc.c
> @@ -98,7 +98,7 @@ static void udf_add_free_space(struct super_block
> *sb, u16 partition, u32 cnt)
>                 return;
> 
>         lvid = (struct logicalVolIntegrityDesc *)sbi->s_lvid_bh->b_data;
> -       le32_add_cpu(&lvid->freeSpaceTable[partition], cnt);
> +       le32_add_cpu(lvid->freeSpaceTable + partition, cnt);
>         udf_updated_lvid(sb);
>  }
> 
> diff --git a/fs/udf/ecma_167.h b/fs/udf/ecma_167.h
> index 14ffe27342bc..215d97d7edc4 100644
> --- a/fs/udf/ecma_167.h
> +++ b/fs/udf/ecma_167.h
> @@ -360,9 +360,9 @@ struct logicalVolIntegrityDesc {
>         uint8_t                 logicalVolContentsUse[32];
>         __le32                  numOfPartitions;
>         __le32                  lengthOfImpUse;
>         __le32                  freeSpaceTable[0];
>         __le32                  sizeTable[0];
> -       uint8_t                 impUse[0];
> +       uint8_t                 impUse[];
>  } __packed;
> 
>  /* Integrity Type (ECMA 167r3 3/10.10.3) */
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 379867888c36..a1fc51c2261e 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -2517,8 +2517,8 @@ static unsigned int udf_count_free(struct super_block *sb)
>                         (struct logicalVolIntegrityDesc *)
>                         sbi->s_lvid_bh->b_data;
>                 if (le32_to_cpu(lvid->numOfPartitions) > part) {
> -                       accum = le32_to_cpu(
> -                                       lvid->freeSpaceTable[part]);
> +                       accum = le32_to_cpup(
> +                                       (lvid->freeSpaceTable + part));
>                         if (accum == 0xFFFFFFFF)
>                                 accum = 0;
>                 }
> 
> 
> 
> This version could easily be backported to stable kernels to let them be
> compiled with gcc-10, and then synchronizing with the udftools version of
> the header needs additional changes on top, which do not need to be
> backported.
> 
>        Arnd
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

  reply	other threads:[~2020-05-01 20:48 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-30 21:30 [PATCH 00/15] gcc-10 warning fixes Arnd Bergmann
2020-04-30 21:30 ` Arnd Bergmann
2020-04-30 21:30 ` Arnd Bergmann
2020-04-30 21:30 ` [PATCH 01/15] crypto - Avoid free() namespace collision Arnd Bergmann
2020-05-08  6:06   ` Herbert Xu
2020-04-30 21:30 ` [PATCH 02/15] iwlwifi: mvm: fix gcc-10 zero-length-bounds warning Arnd Bergmann
2020-06-10 12:18   ` Luciano Coelho
2020-04-30 21:30 ` [PATCH 03/15] mwifiex: avoid -Wstringop-overflow warning Arnd Bergmann
2020-05-06  8:43   ` Kalle Valo
2020-04-30 21:30 ` [PATCH 04/15] ath10k: fix gcc-10 zero-length-bounds warnings Arnd Bergmann
2020-04-30 21:30   ` Arnd Bergmann
2020-04-30 21:45   ` Gustavo A. R. Silva
2020-04-30 21:45     ` Gustavo A. R. Silva
2020-04-30 21:44     ` Arnd Bergmann
2020-04-30 21:44       ` Arnd Bergmann
2020-05-04 11:54     ` Kalle Valo
2020-05-04 11:54       ` Kalle Valo
2020-05-04 16:09       ` Gustavo A. R. Silva
2020-05-04 16:09         ` Gustavo A. R. Silva
2020-05-05  4:56         ` Kalle Valo
2020-05-05  4:56           ` Kalle Valo
2020-04-30 21:30 ` [PATCH 05/15] bpf: avoid gcc-10 stringop-overflow warning Arnd Bergmann
2020-05-04 21:06   ` Daniel Borkmann
2020-04-30 21:30 ` [PATCH 06/15] netfilter: conntrack: avoid gcc-10 zero-length-bounds warning Arnd Bergmann
2020-05-10 21:48   ` Pablo Neira Ayuso
2020-04-30 21:30 ` [PATCH 07/15] drop_monitor: work around gcc-10 stringop-overflow warning Arnd Bergmann
2020-05-01 11:28   ` Neil Horman
2020-04-30 21:30 ` [PATCH 08/15] usb: ehci: avoid gcc-10 zero-length-bounds warning Arnd Bergmann
2020-05-01  2:42   ` Alan Stern
2020-05-01 20:06     ` Arnd Bergmann
2020-05-01 20:10       ` Alan Stern
2020-04-30 21:30 ` [PATCH 09/15] udf: avoid gcc-10 zero-length-bounds warnings Arnd Bergmann
2020-04-30 21:54   ` Pali Rohár
2020-05-01 20:30     ` Arnd Bergmann
2020-05-01 20:48       ` Jan Kara [this message]
2020-05-01 20:57       ` Pali Rohár
2020-04-30 21:30 ` [PATCH 10/15] hpfs: avoid gcc-10 zero-length-bounds warning Arnd Bergmann
2020-04-30 21:30 ` [PATCH 11/15] omfs: avoid gcc-10 stringop-overflow warning Arnd Bergmann
2020-04-30 21:30 ` [PATCH 12/15] media: s5k5baf: avoid gcc-10 zero-length-bounds warning Arnd Bergmann
2020-04-30 21:46   ` Gustavo A. R. Silva
2020-04-30 21:30 ` [PATCH 13/15] scsi: sas: " Arnd Bergmann
2020-05-01  7:47   ` John Garry
2020-05-01  7:54     ` Arnd Bergmann
2020-05-01 14:53       ` James Bottomley
2020-05-01 17:36         ` Arnd Bergmann
2020-04-30 21:30 ` [PATCH 14/15] isci: " Arnd Bergmann
2020-04-30 21:30 ` [PATCH 15/15] nvme: " Arnd Bergmann
2020-04-30 21:30   ` Arnd Bergmann
2020-05-01  7:32   ` Christoph Hellwig
2020-05-01  7:32     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200501204843.GA9544@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=jack@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=steve@digidescorp.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.