[PATCH 32/38] fs: fat: Shrink the size of a few strings

Ilias Apalodimas ilias.apalodimas at linaro.org
Fri Mar 31 09:33:47 CEST 2023


On Fri, 31 Mar 2023 at 09:51, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
>
>
> Am 31. März 2023 03:16:13 MESZ schrieb Simon Glass <sjg at chromium.org>:
> >Hi Heinrich,
> >
> >On Fri, 31 Mar 2023 at 13:05, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >>
> >>
> >> Am 31. März 2023 01:49:35 MESZ schrieb Simon Glass <sjg at chromium.org>:
> >> >Hi Heinrich,
> >> >
> >> >On Fri, 31 Mar 2023 at 11:48, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >> >>
> >> >>
> >> >>
> >> >> Am 30. März 2023 23:32:21 MESZ schrieb Simon Glass <sjg at chromium.org>:
> >> >> >To save a few bytes, replace Error with ** and try to use the same string
> >> >> >for multiple messages where possible.
> >> >> >
> >> >> >Signed-off-by: Simon Glass <sjg at chromium.org>
> >> >> >---
> >> >> >
> >> >> > fs/fat/fat.c       | 12 ++++++------
> >> >> > fs/fat/fat_write.c | 14 ++++----------
> >> >> > 2 files changed, 10 insertions(+), 16 deletions(-)
> >> >> >
> >> >> >diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> >> >> >index 2da93dae3cf3..f0df7988e172 100644
> >> >> >--- a/fs/fat/fat.c
> >> >> >+++ b/fs/fat/fat.c
> >> >> >@@ -97,8 +97,8 @@ int fat_register_device(struct blk_desc *dev_desc, int part_no)
> >> >> >       /* Read the partition table, if present */
> >> >> >       if (part_get_info(dev_desc, part_no, &info)) {
> >> >> >               if (part_no != 0) {
> >> >> >-                      printf("** Partition %d not valid on device %d **\n",
> >> >> >-                                      part_no, dev_desc->devnum);
> >> >> >+                      printf("** Partition %d invalid on device %d **\n",
> >> >> >+                             part_no, dev_desc->devnum);
> >> >> >                       return -1;
> >> >> >               }
> >> >> >
> >> >> >@@ -168,7 +168,7 @@ static __u32 get_fatent(fsdata *mydata, __u32 entry)
> >> >> >       __u32 ret = 0x00;
> >> >> >
> >> >> >       if (CHECK_CLUST(entry, mydata->fatsize)) {
> >> >> >-              printf("Error: Invalid FAT entry: 0x%08x\n", entry);
> >> >> >+              printf("** Invalid FAT entry: %#08x\n", entry);
> >> >>
> >> >> The ** is superfluous. The text makes it clear that an error occured
> >> >
> >> >So should I drop the other ** strings in these files too? Please take
> >> >a look and see what you think.
> >>
> >> I suggest to avoid prefixes like 'Error:' and '**' in all our code if the message text already indicates an error.
> >
> >That makes sense to me. It is sometimes hard to know whether something
> >indicates an error, though. If you look through fat.c you'll see what
> >I mean.
>
> Users might prefer log_err writing messages in red on the console.

FWIW I agree with Heinrich here.  We got 'Error: XXXXX' sprinkled
around from older code, but it makes sense to convert them to log_err
where possible

Regards
/Ilias
>
> >
> >Regards,
> >Simon


More information about the U-Boot mailing list