lkml.org 
[lkml]   [2020]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 09/15] udf: avoid gcc-10 zero-length-bounds warnings
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

\
 
 \ /
  Last update: 2020-05-01 22:49    [W:0.095 / U:22.408 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site