Re: [PATCH] Subject:[PATCH] cxl-cdat:Fix open file not closed in ct3_load_cdat

[Hao Zeng]

On Tue, 2023-04-11 at 16:39 +0100, Peter Maydell wrote:
Dear Peter
Thank you for taking the time to reply to my email. I appreciate your
the valuable information you have provided.

> On Mon, 3 Apr 2023 at 13:51, zenghao <zenghao@kylinos.cn> wrote:
> > 
> > opened file processor not closed,May cause file processor leaks
> > 
> > Fixes:aba578bdace5303a441f8a37aad781b5cb06f38c
> > 
> > Signed-off-by: Zeng Hao <zenghao@kylinos.cn>
> > Suggested-by: Xie Ming <xieming@kylinos.cn>
> > ---
> >  hw/cxl/cxl-cdat.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
> > index 137abd0992..ba7ed1aafd 100644
> > --- a/hw/cxl/cxl-cdat.c
> > +++ b/hw/cxl/cxl-cdat.c
> > @@ -128,6 +128,7 @@ static void ct3_load_cdat(CDATObject *cdat,
> > Error **errp)
> > 
> >      if (fread(cdat->buf, file_size, 1, fp) == 0) {
> >          error_setg(errp, "CDAT: File read failed");
> > +        fclose(fp);
> >          return;
> >      }
> 
> Coverity also spotted this, as CID 1508069.
> 
> The other 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". This check should be
>  if (fread(cdat->buf, file_size, 1, fp) != file_size) {
>     ...
>  }
> I think.
>  
> thanks
> -- PMM
I couldn't agree more with your thoughts.
I will fix the bug in a separate commit.(CID 1507822)

Best regards
Hao