<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Sep 7, 2013 at 6:41 PM, Rob Landley <span dir="ltr"><<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 09/03/2013 03:00:50 AM, Ashwini Sharma wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Rob,<br>
<br>
Attached is the patch for _fsck_ toy.<br>
<br>
This will parse _/etc/fstab_ for devices and their properties and then does<br>
a _fork_ _exec_ for the<br>
fsck.<type> tool.<br>
</blockquote>
<br></div>
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.<br></blockquote><div><br></div><div>I had considered using xgetmountlist(), but that didn't have all the fields from /etc/fstab like passno, mntopts.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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?<br></blockquote>
<div><br></div><div>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.</div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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?)<br>

<br></blockquote><div><br></div><div>fsck man page had a mention of these environment variables and it is also the use case at my end.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

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.<br>

<br>
So no, the copy isn't necessary, meaning the free function isn't necessary. It can just llist_traverse(&list, free);</blockquote><div><br></div><div>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> ...</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Have a look at it and let me know for your comments.<br>
</blockquote>
<br></div>
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.)<br>

<br>
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.<br>

<br>
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.)<br>

<br>
Sorry I'm behind on email right now...<span class="HOEnZb"><font color="#888888"><br>
<br>
Rob</font></span></blockquote></div><br></div><div class="gmail_extra">regards,</div><div class="gmail_extra">Ashwini</div></div>