[Toybox] - Add fsck

[Ashwini Sharma]

On Sat, Sep 7, 2013 at 6:41 PM, Rob Landley <rob at landley.net> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20130907/07b8c7bd/attachment-0006.htm>