At 14:18 11/07/02, Szakacsits Szabolcs wrote:
>I plan to commit the below [with comments] to volume.[ch] for general
>use and also convert ntfsfix.c to use this.
cool, see below for a few comments.
>However ntfsfix also accepts any version 2.X. Does that really needed?
Yeah, Please adapt your function to accept all 2.x versions, too. (And
always treat them in your mind as if they were 3.x, but don't worry too
much as the probability of anyone ever running one of our utilities on a
system with NTFS 2.x is so tiny it is not worth thinking about!)
>I also plan to move log journal reset and
Either logfile.[hc] or volume.[hc], not sure what is appropriate... you
decide...
Please, only one argument: ntfs_volume *vol, function does rest. Also,
please don't use the code from ntfsfix and instead use ntfs_open_inode() +
ntfs_open_attr() [you now know the size] + loop using ntfs_attr_pwrite()
with a 4kiB or so buffer which is memset to -1 over the length of the
attribute + close the attr and the inode. I have been meaning to do this
for ages but never got round to it and ntfs_attr_pwrite() was not advanced
enough back then, but it is now... Thanks! (-:
>marking volume dirty to the library.
volume.[hc]... For the volume flags, perhaps instead of having a function
"ntfs_set_volume_dirty()" or whatever, it would be better to have a
function "ntfs_set_volume_flags(ntfs_volume *vol, VOLUME_FLAGS flags)" and
let the function do everything... You need to preserve old flags so when
using the function, you would need to call it with (vol, vol->flags |
VOLUME_IS_DIRTY).
Of course you could then have a static inline ntfs_set_volume_dirty(vol)
function in volume.h just wrapping doing a simple "return
ntfs_set_volume_flags(vol, vol->flags|VOLUME_IS_DIRTY);"...
Somewhere, we will need to set the VOLUME_MOUNTED_ON_NT4 flag if ntfs
major > 1. Not sure when this should happen. Possibly at mount time if not
mounted MS_RDONLY? It would be a convenient place as we already read the
flags, so it is easy to or this bit and write the mft record straight
back... Of course we wrap this with an if ntfs major > 1...
Further I think the journal emptying should actually happen at ntfs_mount()
time, perhaps we could have a mount option to prevent this from happening?
Although I don't see why would want to prevent it, so probably no need to
bother. Again, we would only do this if not mountind MS_RDONLY.
How does that sound?
>Any objection?
No. (-:
>#define NTFS_V1_1(major, minor) ((major) == 1 && (minor) == 1)
>#define NTFS_V1_2(major, minor) ((major) == 1 && (minor) == 2)
>#define NTFS_V3_0(major, minor) ((major) == 3 && (minor) == 0)
>#define NTFS_V3_1(major, minor) ((major) == 3 && (minor) == 1)
>
>int ntfs_version_supported(u8 major, u8 minor)
how about
BOOL ntfs_is_version_supported(ntfs_volume *vol)
and return TRUE or FALSE... no need for errno... fits in with
is_ntfs_bootsector() [or whatever it is called, can't remember exactly]
>{
>
> if (NTFS_V1_1(major, minor) || NTFS_V1_2(major, minor))
> return 0;
>
> if (NTFS_V3_0(major, minor) || NTFS_V3_1(major, minor))
> return 0;
>
> errno = ENOTSUP;
> return -1;
>}
Looks fine. I don't really mind if you go your way or with my
suggestions... The only thing I would like to definitely be added is
NTFS_V2_x() to be a valid version. Thanks.
Anton
--
"I've not lost my mind. It's backed up on tape somewhere." - Unknown
--
Anton Altaparmakov <aia21 at cantab.net> (replace at with @)
Linux NTFS Maintainer / IRC: #ntfs on irc.openprojects.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
|