On Sat, Jul 19, 2003 at 04:50:42PM +0200, Marco Gerards wrote:
> Here is a new version of this patch. I've included some GCS related
> updates and removed something that is not relevant form the last
> patch. I've also changed the changelog entry a bit so the changes are
> grouped.
Great, thanks for this. I have a couple of remarks. Please consider them
and then upload the reviewed patch to Savannah.
> 2003-07-07 Marco Gerards <[EMAIL PROTECTED]>
>
> * node-create.c: New file.
It would be nice to have a comment in that file (like, before the actual
content) why we need to override this from libdiskfs. IE, explain the main
difference in operation.
> * Makefile (SRCS): Added node-created.c.
> * dir.c: Include <hurd/fsys.h>.
> (diskfs_direnter_hard): Initialize a new block with zeros.
> (diskfs_direnter_hard): Enter direntry and setup the virtual
> inode. Also handle directories correctly.
We don't repeat the function name this way, just list all changes in one
entry.
> (diskfs_rewrite_hard): Function rewritten.
>
> (diskfs_dirempty): Change logic to test if a file was deleted.
I don't like the empty lines here, with no new "* dir.c". Either they are
different changes and require a new "*", or they belong together. In this
case, I suggest to just lump everything together. They are lots of small
cleanups for one goal, write support.
> * fat.c (fat_extend_chain): Move spin_lock to prevent deadlock.
This one is wrong, you must not access dn->length_of_chain without holding
the lock (the caller only has the reader lock of alloc_lock). So just
unlock the lock before the "return 0" in the if (because that is the bug you
were seeing, I guess).
> (fat_extend_chain): Set dn->last to 0 when deallocating the
> complete file.
> (fat_extend_chain): Update dn->last when not deallocating the
> complete file.
> (fat_extend_chain): Set dn->first to zero when the complete file
> was deallocated. Also update dn->length_of_chain to the new amount
> of clusters in the chain.
Again, just lump all in one (fat_extend_chain).
> * main.c (diskfs_readonly): Remove global variable.
> (diskfs_hard_readonly): Likewise.
>
> Common subdirectories: /home/marco/src/hurdcvs/hurd/fatfs/CVS and fatfs/CVS
> diff -BNup /home/marco/src/hurdcvs/hurd/fatfs/dir.c fatfs/dir.c
> --- /home/marco/src/hurdcvs/hurd/fatfs/dir.c 2003-05-10 02:12:29.000000000 +0200
> +++ fatfs/dir.c 2003-07-19 18:43:14.000000000 +0200
> @@ -1,5 +1,7 @@
> -/* main.c - FAT filesystem.
> - Copyright (C) 1997, 1998, 1999, 2002 Free Software Foundation, Inc.
> +/* dir.c - Directory management routines.
> + Copyright (C) 1997, 1998, 1999, 2002, 2003 Free Software
> + Foundation, Inc.
Try hard to get it in one line, by removing the spaced between the years.
Also watch your coding style. I have seen this (two spaces):
+ node->dn->length_of_chain = clusters_to_keep;
:)
Thanks,
Marcus
--
`Rhubarb is no Egyptian god.' GNU http://www.gnu.org [EMAIL PROTECTED]
Marcus Brinkmann The Hurd http://www.gnu.org/software/hurd/
[EMAIL PROTECTED]
http://www.marcus-brinkmann.de/
_______________________________________________
Bug-hurd mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-hurd
