On 14 June 2014 11:57, Tobias Stoeckmann <[email protected]> wrote:
> On Sat, Jun 14, 2014 at 11:25:22AM -0400, Kenneth Westerback wrote:
>> >                 /* follow the chain to its end (hopefully) */
>> > -               for (p = head;
>> > +               for (len = fat[head].length, p = head;
>> >                      (n = fat[p].next) >= CLUST_FIRST && n < 
>> > boot->NumClusters;
>> >                      p = n)
>>
>> An assignment inside the condition makes me cry, but I guess that's
>> not related to *this* problem.
>>
>> > -                       if (fat[n].head != head)
>> > +                       if (fat[n].head != head || len-- < 2)
>>
>> The '2' is a bit magical, perhaps worth a comment somewhere.
>
> The assignment is the cause of this magical number.
>
> Even if len would be 0, n would already point to the next entry of the
> linked list, which means it's always one entry ahead.  It bugs me too
> but I tried to keep the diff as less intrusive as possible.
>
> Accepting that len is "off by one", the "len"th position of the
> linked list will be reached when len is 1.  And just to make sure that
> I can cover the len=0 case, I check for < 2 and decrement afterwards,
> otherwise the unsigned value would overflow.
>
> Although fat[head].length = 0 shouldn't be possible ...

I figured it out after some head scratching and trying to come up with
a better loop. As you hint, this become very intrusive very quickly.
:-)

What I was trying to say was that a comment in the code would be
helpful for the next spelunker to come along.

Still ok krw@

..... Ken

>
>
> You can fetch a broken filesystem from my website (1 MB), it also covers
> the case in which two FATs don't match.  FAT 1 contains the loop for
> cluster chain from 106 to 110, whereas 110 links back to 106:
> http://www.stoeckmann.org/openbsd/poc.iso
>
> Output of this will be:
>
> $ ftp http://www.stoeckmann.org/openbsd/poc.iso
> # vnconfig vnd0c poc.iso
> # fsck_msdos vnd0c
> ** /dev/rvnd0c (vnd0c)
> ** Phase 1 - Read and Compare FATs
> Cluster 110 is marked as EOF in FAT 0, but continues with cluster 106 in FAT 1
> Use continuation from FAT 1? [Fyn] y
> ** Phase 2 - Check Cluster Chains
> Cluster chain starting at 106 loops at cluster 110
> Clear chain starting at 106? [Fyn] n
> Truncate? [Fyn] y
> ** Phase 3 - Check Directories
> ** Phase 4 - Check for Lost Files
> 2 files, 984 free (246 clusters)
>
> ***** FILE SYSTEM WAS MODIFIED *****
> # vnconfig -u vnd0c
> # _
>
>
> Tobias

Reply via email to