Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 3 Apr 2010 17:07:33 +0200
From:      Tijl Coosemans <tijl@coosemans.org>
To:        freebsd-current@freebsd.org
Cc:        Kostik Belousov <kostikbel@gmail.com>, Bruce Evans <bde@zeta.org.au>, Andriy Gapon <avg@freebsd.org>
Subject:   Re: newfs_msdos and DVD-RAM
Message-ID:  <201004031707.34650.tijl@coosemans.org>
In-Reply-To: <4BB64615.9060601@freebsd.org>
References:  <3a142e751003190508x6a06868ene2e8fd9ddd977f66@mail.gmail.com> <4BB644CA.4000807@freebsd.org> <4BB64615.9060601@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 02 April 2010 21:31:33 Andriy Gapon wrote:
> on 02/04/2010 22:26 Andriy Gapon said the following:
>> OK, I did it again.
>> I tested the below patch using the scenario described above.
>> Could you please review and/or test this patch?
>> If you like it and it works, I can commit it.
>> Thanks!
>> 
>> --- a/sbin/newfs_msdos/newfs_msdos.c
>> +++ b/sbin/newfs_msdos/newfs_msdos.c
>> @@ -427,6 +427,9 @@ main(int argc, char *argv[])
>>      if (bpb.bpbBytesPerSec < MINBPS)
>>  	errx(1, "bytes/sector (%u) is too small; minimum is %u",
>>  	     bpb.bpbBytesPerSec, MINBPS);
>> +    bpb.bpbSecPerClust /= (bpb.bpbBytesPerSec / MINBPS);
>> +    if (bpb.bpbSecPerClust == 0)
>> +	bpb.bpbSecPerClust = 1;
>>      if (!(fat = opt_F)) {
>>  	if (opt_f)
>>  	    fat = 12;
>> 
> 
> And here is a safer one (in case of a huge sector size > 32KB).
> I will appreciate any testing with real media that you might have.
> 
> diff --git a/sbin/newfs_msdos/newfs_msdos.c b/sbin/newfs_msdos/newfs_msdos.c
> index 955c3a5..3f2778d 100644
> --- a/sbin/newfs_msdos/newfs_msdos.c
> +++ b/sbin/newfs_msdos/newfs_msdos.c
> @@ -427,6 +427,12 @@ main(int argc, char *argv[])
>      if (bpb.bpbBytesPerSec < MINBPS)
>  	errx(1, "bytes/sector (%u) is too small; minimum is %u",
>  	     bpb.bpbBytesPerSec, MINBPS);
> +    bpb.bpbSecPerClust /= (bpb.bpbBytesPerSec / MINBPS);
> +    if (bpb.bpbSecPerClust == 0)
> +	bpb.bpbSecPerClust = 1;
> +    if (bpb.bpbSecPerClust * bpb.bpbBytesPerSec > 32 * 1024)
> +	errx(1, "bytes per sector (%u) is greater than 32k",
> +	    bpb.bpbSecPerClust * bpb.bpbBytesPerSec);
>      if (!(fat = opt_F)) {
>  	if (opt_f)
>  	    fat = 12;

Wikipedia's article on FAT has this to say about the maximum size of
clusters:

"The limit on partition size was dictated by the 8-bit signed count of
sectors per cluster, which had a maximum power-of-two value of 64. With
the standard hard disk sector size of 512 bytes, this gives a maximum
of 32 KB clusters, thereby fixing the "definitive" limit for the FAT16
partition size at 2 gigabytes. On magneto-optical media, which can have
1 or 2 KB sectors instead of 1/2 KB, this size limit is proportionally
larger.

Much later, Windows NT increased the maximum cluster size to 64 KB by
considering the sectors-per-cluster count as unsigned. However, the
resulting format was not compatible with any other FAT implementation
of the time, and it generated greater internal fragmentation. Windows
98 also supported reading and writing this variant, but its disk
utilities did not work with it."


I'm not sure the second paragraph is worth supporting, but the first
seems to say that 32k limit you have in your patch only applies to
disks with 512 byte sectors. For disks with larger sectors it would
be proportionally larger.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004031707.34650.tijl>