On 11/20/2016 11:01 PM, Laine Stump wrote: > This new function just calls stat() and returns st_size (or -1 if > there is an error). We may decide we want this function to be more > complex, and handle things like block devices - this is a placeholder > (that works) for any more complicated funtion. > > NB: virFileLength() takes a path rather than an fd because it needs to > be called for files that can't be opened (due to permissions). I almost wonder if it would be better to take both a path AND an fd, with semantics of: if fd == -1, use path alone - stat() if fd >= 0, path should be non-NULL and describes the fd (for error messages) - fstat() For block devices, [f]stat() probably won't do what we want, where lseek(SEEK_END) and/or ioctl() may be better. But like you say, those can be added later. I definitely like the idea of having a nice wrapper function that we can enhance later, to abstract out the logic so that it is not repeated in the callers. > --- > New in "V2" > > src/libvirt_private.syms | 1 + > src/util/virfile.c | 13 +++++++++++++ > src/util/virfile.h | 2 ++ > 3 files changed, 16 insertions(+) > > > + > +off_t > +virFileLength(const char *file) > +{ > + struct stat s; > + > + if (stat(file, &s) < 0) > + return -1; > + > + return s.st_size; Since we KNOW it won't work right on block devices, and we have the stat() results, shouldn't this also check for S_ISREG(s.st_mode) and return -1 if not? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list