[BusyBox] Add mkdosfs tool (mkfs.msdos) [PATCH]

[Rob Landley]

On Sunday 25 September 2005 01:34, Mike Frysinger wrote:
> On Tuesday 19 July 2005 12:14 am, Jason Schoon wrote:
> > This is based on 1.00, but I would expect it to apply fairly cleanly
> > to the latest as well.
>
> ive updated to dosfstools-2.11 and the busybox svn trunk ... see if the
> attached patch works for you please
> -mike

Sounds cool, let's see...

This patch adds (badly wordwrapped) help for fuser.  Does it add fuser?

Do we really care about atari format?  (We don't care about kernel 2.2 but we 
care about support for hardware that hasn't been manufactured in 15 years?  I 
mean yay rah that there are tools out there, but is it busybox?)

Bad block support should be optional these days; modern hardware (including 
cheap plastic flash devices) remaps the suckers.

I don't know what "ignore 'full' disks" means.  Or for that matter, why -C 
"create a new file" could possibly be relevant.  (dd if=/dev/zero).

What MKDOSFS_ADVANCED's help says it chops out and what the non-trival usage 
message actually has chopped out are very different things.

Trunate the GPL boilerplate.

Everything from "#include <linux/version.h>" to "#define llseek lseek" is just 
evil.  (Isn't there standard userspace endianness stuff in the network 
headers?)

Does cdiv() have any point at all?  Every single usage of it, the second 
argument is sector_size...

Trace FAT_BAD through mark_sector_bad (the only use) and then look at the two 
uses of mark_sector_bad and go "gee, no opportunity for shared code 
_there_..."

Why isn't MAX_CLUST_12 instead MAX_CLUSTER(12), and yes this means 
MAX_CLUST_32 -> MAX_CLUSTER(28), which deserves a comment but where it's 
_used_ not way up at the top of the file.

__u8 is a kernel type.  Just use c99 types.  And the whole boot_sector 
structure definition is a bit of a mess, and the two #defines right after it 
are unnecessary obfuscation.

The whole hard-wired assembly boot sector thing to say "this is not a bootable 
disk".  How is this an improvement over just writing a bunch of zeroes so we 
get the "non-system disk or disk error" message from the BIOS?  This is a lot 
of (x86-specific) effort put in to accomplish _nothing_.

Busybox already has too many globals in it, which have space allocated for all 
of them in every single applet run by the way.

#ifdef ADVANCED
stuff
#else
stuff
#endif
one line
#ifdef ADVANCED

Ahem.

Our our strings mutable or immutable?  (volume_name[]="     ")

Ok, I can see that blocks is forced to be 64 bits on 32 bit platforms (long 
long) because fat32 has 2<<28 sectors and the maximum sector size is 512<<7 
for 35 bits, BUT the important thing to ask ourselves is do we really _care_ 
about creating FAT volumes greater than four terabytes?  This is FAT we're 
talking about here, it's known for not scaling.  Just adjust the exiting "FAT 
too big" test and be done with it....

I'm going to stop now.  This needs a massive, massive cleanup to be a proper 
busybox applet.  Probably a good week's worth of work to do it right.

Rob