Re: Adding functions to libutil, part 1

[Reinoud Zandijk <reinoud%netbsd.org@localhost>]

Hi Michael,

On Fri, Jun 21, 2024 at 05:50:56AM -0000, Michael van Elst wrote:
> reinoud%netbsd.org@localhost (Reinoud Zandijk) writes:
> 
> >  int getdiskinfo(const char *s, int fd, const char *dt, struct disk_geom *geo,
> >  	struct dkwedge_info *dkw);
> >  int getdisksize(const char *name, u_int *secsize, off_t *mediasize);
> 
> 
> There are kernel functions of the same name:
> 
> int getdiskinfo(struct vnode *vp, struct dkwedge_info *dkw)
> int getdisksize(struct vnode *vp, uint64_t *numsecp, unsigned int *secsizep);
> 
> Maybe these should be aligned somewhat (starting with argument order).

That would be more sane yes; the whole arguments for getdiskinfo are now
(name of device, filehandle, fstab type, disk_geom return, dkwedge_info return).

The name is only used for error printing and it could at least use the
filehandle for fstat() :) The optional disktab entry is a bit ... odd; its
only function seems to be to error out if it not known and the result is not
even used. This argument is only used by newfs{,_ext2fs, _lfs} and I think
this parameter can go.

More sane would be:

  int getdiskinfo(int fd, struct disk_geom *geo, struct dkwedge_info *dkw);

with dkwedge_info still at the end as it is optional. getdisksize() could be
removed as it doesn't have callers in userland anyway.

> IMO moving the functionality into libutil is a good thing. But just
> moving the code (and multiplying its users) is maybe not.

Indeed.

Reinoud