Re: [PATCH v2 2/2] cxl-cdat:Fix the check on the return value of fread()

[Jonathan Cameron via <qemu-devel@nongnu.org>]

On Wed, 12 Apr 2023 12:02:47 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> On 12/4/23 09:16, Hao Zeng wrote:
> > The bug in this code (CID 1507822) is that the
> > check on the return value of fread() is wrong. fread()
> > returns the number of items read or written, so
> > checking for == 0 only catches "no data read at all",
> > not "only read half the data".
> > 
> > Signed-off-by: Zeng Hao <zenghao@kylinos.cn>
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/cxl/cxl-cdat.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
> > index ba7ed1aafd..130531a9cd 100644
> > --- a/hw/cxl/cxl-cdat.c
> > +++ b/hw/cxl/cxl-cdat.c
> > @@ -126,7 +126,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
> >       fseek(fp, 0, SEEK_SET);
> >       cdat->buf = g_malloc0(file_size);  
> 
> Pointless bzero in g_malloc0, however this code would be
> simplified using g_file_get_contents().

Agreed - switching this whole thing to g_file_get_contents()
will get rid of this code and be a lot simpler.
Perhaps just jump directly to that and note the two bugs that existed
in the code that is replaced?

Jonathan
 

> 
> >   
> > -    if (fread(cdat->buf, file_size, 1, fp) == 0) {
> > +    if (fread(cdat->buf, file_size, 1, fp) != file_size) {
> >           error_setg(errp, "CDAT: File read failed");
> >           fclose(fp);
> >           return;  
>