Re: [PATCH] AFS fixes and improvements

[Vladimir 'phcoder' Serbinenko]

On Sun, Jul 19, 2009 at 7:11 AM, Pavel Roskin<address@hidden> wrote:
> Quoting Vladimir 'phcoder' Serbinenko <address@hidden>:
>
>> Update: added symlink support
>>
>> On Fri, Jul 17, 2009 at 9:30 PM, Vladimir 'phcoder'
>> Serbinenko<address@hidden> wrote:
>>>
>>> Hello. Currently I'm coding BFS (filesystem of BeOS and Haiku) which
>>> is similar to AFS. So I took the later as codebase. I found some bugs
>>> and incompletenesses in it. Here is the fix. Tested using grub-fstest
>>> on virtual machine image downloaded from Syllable website and then
>>> successfully booted from it using grub-mkrescue image
>
> Many changes have no value at all, e.g.:
>
> -#define        GRUB_AFS_SBLOCK_MAGIC1  0x41465331      /* AFS1 */
> +/* AFS1 in hexadecimal.  */
> +#define        GRUB_AFS_SBLOCK_MAGIC1  0x41465331
>  #define        GRUB_AFS_SBLOCK_MAGIC2  0xdd121031
>  #define        GRUB_AFS_SBLOCK_MAGIC3  0x15b6830e
>
> The original comment suggested that only the first value made it clear that
> only the first value is AFS1.  Now it's unclear.  There is no need to point
> out that AFS1 is in hexadecimal notation.  It's obvious to anyone competent
> to read C code.  Besides, there is no need to single out AFS1.
>
I thought GCS mandated against putting comments together with field
but I was wrong
> -  if ((! dir->inode.stream.size) ||
> +  if ((dir->inode.stream.size == 0) ||
>
> The later is marginally better, but it would be easier to review your
> patches if you don't include such changes.
It's not a stylistic improvement. dir->inode.stream.size is 64-bit
quantity which will be truncated on 32-bit platform
>
> Double semicolons can be removed in all files, and it doesn't need a review.
I'll remove all wrong double semicolons now
>
> It's better to split fixes from the new features.
On it
>
> I don't have afs images around.  It would be great if you test all new
> functionality with valgrind.  It's very good at finding mistakes in the
> code.
>
I will do. I use the publically-available image from
http://web.syllable.org/pages/get-Syllable.html#emulate
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git