Discussion:
Adding functions to libutil, part 1
(too old to reply)
Reinoud Zandijk
2024-06-20 20:11:52 UTC
Permalink
Dear folks,

i'd like to add the following files to libutil since they are referenced
multiple times all reaching over to fsck while they are not fsck specific.

sbin/fsck/partutil.h
sbin/fsck/partutil.c

which define the following functions:

struct dkwedge_info;
struct disk_geom;

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);

getdiskinfo() returns as might be expected the disc geometry and wedgeinfo (if
available). getdisksize() returns the sector size and the total media size.

Are there any objections to this inclusion in libutil (with their manpages of
course) ?

With regards,
Reinoud


partutil.h is included in
sbin/fsck/partutil.c
sbin/fsck_ext2fs/setup.c
sbin/fsck_ffs/setup.c
sbin/newfs/newfs.c
sbin/newfs_ext2fs/newfs_ext2fs.c
sbin/newfs_lfs/newfs.c
sbin/newfs_msdos/mkfs_msdos.c
sbin/newfs_udf/udf_core.c
sbin/resize_lfs/resize_lfs.c
usr.sbin/sysinst/geom.c


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Michael van Elst
2024-06-21 05:50:56 UTC
Permalink
Post by Reinoud Zandijk
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).


Taking a name instead of a filehandle in fsck's getdisksize()
doesn't look that generic to me. The fsck code for getdiskinfo() is
also accessing the file through the handle and calling stat() on the
name.

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



--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Reinoud Zandijk
2024-06-21 10:52:32 UTC
Permalink
Hi Michael,
Post by Michael van Elst
Post by Reinoud Zandijk
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);
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.
Post by Michael van Elst
IMO moving the functionality into libutil is a good thing. But just
moving the code (and multiplying its users) is maybe not.
Indeed.

Reinoud


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...