Re: [Xen-devel] [PATCH V4 2/7] libxl_read_file_contents: add new entry to read sysfs file

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Chun Yan Liu writes ("Re: [Xen-devel] [PATCH V4 2/7] libxl_read_file_contents: 
add new entry    to read sysfs file"):
> <21881.46148.466883.923039@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
> <Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> > Chunyan Liu writes ("[Xen-devel] [PATCH V4 2/7] libxl_read_file_contents: 
> > add  
> > What is this for ? 
> 
> I found sometimes reading sysfs file contents, at the end, there is
> some random character. With this line, there is no problem then.

I'm afraid that's not really a complete explanation.

Guessing, I think your calling code expected a trailing nul.

If you want to make an API which is useable for such code, and not
intended for byte blocks, that's fine, but you must then always
nul-terminate (not only if the data was less than 4k) and your new
function probably doesn't want to return a length at all.

> > Is there any risk that the file is actually bigger than advertised, 
> > rather than smaller ? 
> 
> For sysfs file, couldn't be bigger.

Then you should detect the condition that the file is bigger, and call
it an error.

> > > +int libxl_read_sysfs_file_contents(libxl_ctx *ctx, const char *filename, 
> > > +                                   void **data_r, int *datalen_r); 
> >  
> > I think this is a function with sufficiently odd semantics, and a 
> > sufficiently internal purpose, that it should probably not be exposed 
> > in the API. 
> 
> So move to libxl_internal.h? Or not hacking here but just adding an internal
> function in libxl_pvusb.c with repeated codes?

I meant to move it to libxl_internal.h.  Subject to consideration of
what its API ought to be.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel