+Sean Anderson too, for this Hi,
On Fri, 16 Aug 2024 at 07:36, <[email protected]> wrote: > > From: Johannes Kirchmair <[email protected]> > > Returning negative values from spl_fit_read leads to u-boot crashing. > The return value of spl_fit_read is compared with an unsigned value. > Returning negative values leads to the check not detecting the error. > Not detecting the error leads to crashing. > > Returning zero in case of an reading error is fine. It indicates that > nothing was red. read > > Signed-off-by: Johannes Kirchmair <[email protected]> > --- > common/spl/spl_fat.c | 2 +- > include/spl_load.h | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > Perhaps instead this should be fixed in _spl_load()? We don't want to ignore errors, and returning -EIO is not as good as returning the actual error received. The docs of struct spl_load_info indicate that functions should return 0 on error, but that is somewhat surprising behaviour, which is probably why people are not following it? > diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c > index bd8aab253a..345bc55149 100644 > --- a/common/spl/spl_fat.c > +++ b/common/spl/spl_fat.c > @@ -53,7 +53,7 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong > file_offset, > > ret = fat_read_file(filename, buf, file_offset, size, &actread); > if (ret) > - return ret; > + return 0; > > return actread; > } > diff --git a/include/spl_load.h b/include/spl_load.h > index 1c2b296c0a..b411d9daa1 100644 > --- a/include/spl_load.h > +++ b/include/spl_load.h > @@ -17,8 +17,8 @@ static inline int _spl_load(struct spl_image_info > *spl_image, > { > struct legacy_img_hdr *header = > spl_get_load_buffer(-sizeof(*header), sizeof(*header)); > - ulong base_offset, image_offset, overhead; > - int read, ret; > + ulong base_offset, image_offset, overhead, read; > + int ret; > > read = info->read(info, offset, ALIGN(sizeof(*header), > spl_get_bl_len(info)), header); > -- > 2.34.1 > Regards, Simon
