On Sat, Apr 27, 2024 at 3:22 AM Marek Vasut <ma...@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 <ma...@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
