Re: [PATCH] part: return -ENOSYS when get_info not valid.

[schspa <schspa@gmail.com>]

在 2021-10-19星期二的 17:23 +0200，Heinrich Schuchardt写道：
> On 10/19/21 15:35, zhaohui.shi wrote:
> > From: schspa <schspa@gmail.com>
> > 
> > In some case, get_info() interface can be NULL, add this check to
> > stop
> > from crash.
> 
> Thank you for reviewing the partition driver code.
> 
> There seems to be no driver missing a get_info implementation. Where
> did
> you run into a problem?
> 
Yes, I do run into a problem, In my spl build, CONFIG_SPL_FS_EXT4,
CONFIG_SPL_FS_FAT, CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION are all
not enabled. In this case, get_info implementation is NULL. see
'part_get_info_ptr' and 'part_print_ptr' and part_efi.c for detail.

> Why should we only check .get_info and not .test and not .print?
> 

All part type driver have .test implementation, it can't be NULL,
and .print have NULL pointer judgement already.

> > 
> > Signed-off-by: schspa <schspa@gmail.com>
> 
> Please, provide a name.
> 
> > ---
> >   disk/part.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git a/disk/part.c b/disk/part.c
> > index a6a8f7052b..7af3240ec7 100644
> > --- a/disk/part.c
> > +++ b/disk/part.c
> > @@ -668,6 +668,13 @@ int part_get_info_by_name_type(struct blk_desc
> > *dev_desc, const char *name,
> >         part_drv = part_driver_lookup_type(dev_desc);
> >         if (!part_drv)
> >                 return -1;
> > +
> > +       if (!part_drv->get_info) {
> > +               PRINTF("## Driver %s does not have the get_info()
> > method\n",
> > +                      part_drv->name);
> 
> Please, use log_debug() to avoid noise on the console on every boot.
> 

I think it's OK to use PRINTF, because of this BUG occurs only when
user give a bad part configuration, and this error message can prompt
the user that a configuration error has occurred.
Besides, 'part_get_info' use PRINTF for this null pointer protection
too.

> Best regards
> 
> Heinrich
> 
> > +               return -ENOSYS;
> > +       }
> > +
> >         for (i = 1; i < part_drv->max_entries; i++) {
> >                 ret = part_drv->get_info(dev_desc, i, info);
> >                 if (ret != 0) {
> > 

-- 
Best regards

schspa <schspa@gmail.com>