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

["Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>]

Hi Jan,

Jan Klötzke <jan.kloetzke@xxxxxxxxxx> wrote:
> 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...
[...]

Thanks for the patch!
I've applied it now, but I have some comments:
* Your patch caused several warnings when compiled - you should always
make sure that all warnings are resolved before submitting a patch.
* while we inherited dos fs from Be, and it doesn't exactly follow our
coding style, new code and changes should always do so - I've corrected
your patch in this regard, and cleaned up dos fs a bit while I was at
it
* and:

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

I've not applied this patch - while it solves the problem of
transaction IDs getting negative, there are further problems that would
have to be resolved when the IDs overlap; I think it should be either
handled correctly everywhere (in that component) or not at all.
The latter is even acceptable for now (and several kernel components do
not handle this case gracefully); since Haiku is not a server OS, it
would probably never get a chance to overflow these IDs anyway (bugs
aside).
So while patches that addresses these issues are always welcome, IMO
it's acceptable for R1 to keep suffering from them.

But anyway, thanks a lot!

Bye,
   Axel.