[PATCH v3] mmc: allow use of hardware partition names for mmc partconf

[E Shattow]

On Sat, Apr 27, 2024 at 3:22 AM Marek Vasut <marex at denx.de> wrote:
>
> On 4/27/24 3:29 AM, E Shattow wrote:
> > Hi Marek,
> >
> > On Fri, Apr 26, 2024 at 5:49 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> [...]
> >>
> >>> diff --git a/include/mmc.h b/include/mmc.h
> >>> index 4b8327f1f93b..7243bd761202 100644
> >>> --- a/include/mmc.h
> >>> +++ b/include/mmc.h
> >>> @@ -381,6 +381,21 @@ enum mmc_voltage {
> >>>    #define MMC_TIMING_MMC_HS200        9
> >>>    #define MMC_TIMING_MMC_HS400        10
> >>>
> >>> +/* emmc hardware partition values */
> >>> +enum emmc_hwpart {
> >>> +     EMMC_HWPART_DEFAULT = 0,
> >>> +     EMMC_HWPART_BOOT0 = 1,
> >>> +     EMMC_HWPART_BOOT1 = 2,
> >>> +     EMMC_HWPART_GP1 = 3,
> >>> +     EMMC_HWPART_GP2 = 4,
> >>> +     EMMC_HWPART_GP3 = 5,
> >>> +     EMMC_HWPART_GP4 = 6,
> >>> +     EMMC_HWPART_USER = 7,
> >>> +};
> >>> +
> >>> +/* emmc hardware partition names */
> >>> +extern const char *emmc_hwpart_names[];
> >>
> >> Maybe the array should have fixed size here, i.e. 8 ?
> >
> > Is there an ABI reason to do so? Can you explain further why it would
> > be needed to do that?
>
> It has nothing to do with ABI, it is only to let the compiler validate
> that nobody would index the array with index > 7 by accident.

At least GCC knows this without doing its work again ourselves. How
about a const for the upper limit, where currently EMMC_HWPART_USER
substitutes for expressing an upper limit? You may as well be writing
EMMC_HWPART_MAX = 8 in the enum and using that for the initializer
also any iterators with a less-than condition to make it more
expressive.

$ gcc -o testobj test.c -Wall -Wextra -O2
test.c: In function ‘main’:
test.c:16:5: warning: array subscript 8 is above array bounds of
‘const char *[8]’ [-Warray-bounds=]
   16 |     printf(emmc_hwpart_names[8]);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.c:3:13: note: while referencing ‘emmc_hwpart_names’
    3 | const char *emmc_hwpart_names[] = {
      |             ^~~~~~~~~~~~~~~~~

#include <stdio.h>

const char *emmc_hwpart_names[] = {
       "user",
       "boot0",
       "boot1",
       "gp1",
       "gp2",
       "gp3",
       "gp4",
       "user",
};

int main(int argc, char** argv) {
    (void) argc; (void) argv;
    printf(emmc_hwpart_names[8]);

    return 0;
}

Sorry to belabor this and I'm not a professional programmer. Best regards, -E