FreeLists / haiku-development / [haiku-development] Re: dosfs fixes

["Hugo Santos" <hugosantos@xxxxxxxxx>]

Hi Jan,

 Thanks for the patch. I've looked at it and it seems fine to me. The
only nitpick is probably the one line if statement where you put the
expression after the if instead of below, but that is minimal.

 I'll leave this patch for one of the filesystem folks to apply, i
just wanted to give a thumbs up.

 Hugo

On 5/22/07, Jan Klötzke <jan.kloetzke@xxxxxxxxxx> wrote:
> Hi,
>
> I found a number of bugs when trying to mount a FAT partition from within
> Haiku. The attached diff which contains all changes I made to (hopefully) fix
> those bugs. Now here comes what I sumbled across...
>
> * DOS-FS not mountable readonly
>
> Whenever you try to mount a FAT partition read only (mount -ro) you'll get a
> panic with "tried to get a writeable lock on a read only cache".
>
> Even if mounted read only the function block_cache_get_writable_etc() is
> called by dosfs during directory traversal and causes a panic. In this case
> block_cache_get_etc() should be called I think.
>
> * Sometimes kernel panic with "cache_end_transaction(): invalid transaction
> ID" when browsing through a mounted FAT partition.
>
> The problem is caused by dosfs_readdir() which does not check if diri_init()
> fails. Sometimes (if dosfs_readdir() is called when already at the end of the
> directory?) diri_init() does not start a new transaction and returns NULL.
> Afterwards diri_free() is called but as no transaction was started you will
> go to KDL with the above message.
>
> I think this is equivilent to ticket #1025. In the attached diff I also
> changed other places to check for a failed diri_init().
>
> * dosfs_access() is testing for wrong flags.
>
> Instead of testing for "O_RDONLY" the function must test for "W_OK".
>
> * Block cache transaction id returned by cache_start_transaction() my wrap to
> negative numbers.
>
> Looking at the function it seems that a negative return value should indicate
> a error code. Normal transaction id's should therefore be always positive.
> Furthermore it seems not every caller checks if this function failed.
>
> /Jan