On Sat, Sep 7, 2013 at 6:41 PM, Rob Landley <[email protected]> wrote:
> On 09/03/2013 03:00:50 AM, Ashwini Sharma wrote: > >> Hi Rob, >> >> Attached is the patch for _fsck_ toy. >> >> This will parse _/etc/fstab_ for devices and their properties and then >> does >> a _fork_ _exec_ for the >> fsck.<type> tool. >> > > I'm in the process of adding the /etc/fstab parsing code for umount (for > user mounts in suid mode), in a way it can be shared by mount. This looks > like it needs the same code. > I had considered using xgetmountlist(), but that didn't have all the fields from /etc/fstab like passno, mntopts. > > strtol_range() is similar to what lib/args.c does, presumably there's > common code to be had there. But the only call to strtol_range() has the > range 0, INT_MAX so I'm not sure why it's there? > In lib/args.c are you referring to atolx(), this one doesn't check the overflow errors. Aren't we bothered about such overflows while parsing optflags in toybox. strtol_range can be replaces with get_int_value() from lib/pending.c. I have a cleaned-up version of get_int_value, will share it. > Also, are the getenv(FSCK_MAX_INST) and getenv(FSTAB_FILE) really > necessary? (There's no spec, are the functionality either one provides > actually something toybox needs to worry about? I dunno your use case, did > you implement because you actually need it or just because it was there?) > > fsck man page had a mention of these environment variables and it is also the use case at my end. > list_free() exists because ->data is a separate allocation, let's see if > that's actually necessary... The only caller is free_all() feeding it > TT.devices, which is initialized in fsck_main() assigning an xstrdup() to > data. The field it's duplicating is environment data that lives for the > duration of the function, so the only reason to dup it is if it's modified. > Is it modified? Not through TT.devices at the end of fsck_main() dev = > TT.devices and then dev-> is strcmped (not modifying it) and then assigned > to mt.mnt_fsname which is passed to create_db() which... does another > strdup() on it. > > So no, the copy isn't necessary, meaning the free function isn't > necessary. It can just llist_traverse(&list, free); the source string is modified just after the xstrdup, as the toys.optargs are used to reconstruct the argument list for file system check utility, e.g fsck.ext4 <options> ... > > Have a look at it and let me know for your comments. >> > > Not so trivial I can clean it up right here and now. Reluctant to add yet > more to the giant heap of unreviewed stuff that pending has turned into. > (Yes, it can be in pending for months and then get completely rewritten.) > > My talk outline for Ohio Linuxfest is due today, so I need to do that > instead of cleaning this up at the moment. And when I do get back to > cleanup I'm 2/3 of the way through the tail stuff in response to Felix. And > _then_ I need to finish umount and the cleanup of the ipv6 bits of ifconfig > so I can cut a toybox release so I can cut an Aboriginal Linux release > using the 3.11 kernel. > > Musl maintainer Rich Felker is also giving a talk at Ohio Linuxfest, and I > was hoping to have my aboriginal linux ccwrap rewrite done by the time I > get off the plane thursday so I could go over it with him in person. (I > might delay the aboriginal linux release for that, dunno yet.) > > Sorry I'm behind on email right now... > > Rob regards, Ashwini
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
