Hello,
Mikhail Karpov, le sam. 07 nov. 2026 19:03:56 +0700, a ecrit:
> I also examined the code in corresponding files within libdiskfs
> and found minor differences in the implementation of certain
> functions. It might be worth investigating these areas for potential
> errors.
Yes. That's why duplicating code is usually a bad idea, and if it really
needs to be done (which is the case between libdiskfs and libnetfs), we
need to keep them similar so it's easy to keep them evolve together, and
when a patch is made to one, we need to ask the submitter to do the same
in the other.
> In libnetfs/dir-lookup.c, iohelp_free_iouser (user) is called only in the
> event of an error
>
> err = iohelp_create_empty_iouser (&user);
> if (! err)
> {
> newpo = netfs_make_peropen (dnp, 0, dircred->po);
> if (! newpo)
> {
> err = errno;
> iohelp_free_iouser (user);
> }
> else
> {
> newpi = netfs_make_protid (newpo, user);
> if (! newpi)
> {
> err = errno;
> iohelp_free_iouser (user);
> }
> else
> newpo = NULL;
> }
> }
>
> while in libdiskfs/dir-lookup.c, iohelp_free_iouser (user) is called
> regardless of the outcome of diskfs_make_peropen and diskfs_create_protid.
>
> err = iohelp_create_empty_iouser (&user);
> if (! err)
> {
> err = diskfs_make_peropen (dnp, 0, dircred->po, &newpo);
> if (! err)
> {
> err = diskfs_create_protid (newpo, user, &newpi);
> if (! err)
> newpo = 0;
> }
>
> iohelp_free_iouser (user);
> }
Yes, in diskfs_finish_protid a duplicate of user is taken, while
netfs_make_protid records the pointer.
One reason why diskfs_create_protid takes a duplicate is
probably because it can be given user == NULL. That's used in
libdiskfs/boot-start.c, but these could as well just create a 0,0 iouser
by themselves (or introduce a helper that does it for them).
diskfs_S_fsys_getroot also uses a local variable, but it could as well
just allocate it with iohelp_create_empty_iouser and give it to
diskfs_create_protid.
In the cases where diskfs_create_protid is given dircred->user, we can
make an explicit duplicate, like libnetfs does.
That way, we'll avoid the unnecessary duplicates.
Samuel
