Ognyan Kulev <[EMAIL PROTECTED]> writes: > Marco Gerards wrote: > > Ogi (I hope he is subscribed to this list) wrote some patches for the Hurd so > > ext2fs can support large partitions. It seems to me that not many > > people have looked at these patches :(. Having a >2GB capable ext2fs > > is very important to me. > > The people that commented on this patch are less that the fingers of > one of my hands ;-)
:( > > The thing that bothered me when reading these patches was that byte > > offsets were used quite often. This will make it not possible to use > > partitions bigger than 4GB in some circumstances. > > All used byte offsets should be 64-bit (off_t). My main target was to > _quickly_ release patch that works. That's the reason that I made as > few changes to the original design as possible. Applying ext2fs patch > that works fine is higher priority than improving the code. Right. I just think it is better to use block offsets instead of byte offsets when this is possible. Is off_t always 64 bits or only with FILE_OFFSET_BITS=64? (So if someone compiles ext2fs outside the hurd sourcetree he should set FILE_OFFSET_BITS=64). I understand you did a quick release, I'm glad you did that. :) > > Most likely I forgot some. Especially the pokel stuff might be bad. > > Another use for these functions was to read/write the superblock; > > although it isn't that bad, it would be better if you didn't use byte > > offsets for this. > > Perhaps this problem can be fixed by removing all these functions so > > they can't be used accidently. I see no reason to have these > > functions anyway. > > I will need some days to go deeper about these. Yeah, I even think you do not need pokels at all. I'm not too sure about it though, I should read the other cod first. > > Another thing I've noticed in the patch were changes in the amount of > > whitespaces. This isn't a big problem (for me), it is just something > > to lookout for when sending in the definite patch, here is an example: > > - * We have succeeded in finding a free byte in the block > > - * bitmap. Now search backwards up to 7 bits to find the > > - * start of this group of free blocks. > > + * We have succeeded in finding a free byte in the block > > + * bitmap. Now search backwards up to 7 bits to find the > > + * start of this group of free blocks. > > Yes, there is changed whitespace in some places. In all of them, it's > done the way Emacs do it. In this particular example, I think that * > are aligned to the leading /* or something like that. AFAIR I've made > such whitespace changes in the legacy ext2 code from Linux. Ok, I just wanted to know if it was accidental or not. > > The last thing I mentioned was this: > > +#if 1 > > err = find_block (node, offset, &block, &lock); > > if (err) > > break; > > assert (block); > > +#else > > + /* XXX: I still haven't investigated why this is needed... */ > > + err = ext2_getblk (node, offset >> log2_block_size, 1, &block); > > + if (err) > > + break; > > +#endif > > I'm not really an ext2fs expert, but I think this is used for sparse > > files. Ext2fs doesn't allocate blocks when seeking, it just makes the > > file sparse. So the blocks get allocated just before they are written > > to disk. > > As you see, the original code is compiled by default. The #else is > just garbage, it _was_ required because of some bugs that are gone > now. I believe it can be removed safely, as other #if 0 in the code. Ok. > > I also have a question about the code. The assembly code "int $3" is > > used. AFAIK this is used to tell the debugger a breakpoint is > > reached, right? When is this function (abort) called? > > Well, this is why it is called alpha ;-) There was a time when > assertions failed very often. When they fail, they call abort(), > which I replaced with the function you saw and which contains this > "int $3". This is solely for debugging purposes. Interesting, I didn't know that. > Now, about the future of ext2fs. I won't be able to work on it much > till the end of November. So if you, or anyone else, want to work on > it, it will be best if we put it on hurd-extras or some other public > place where changes can be made by many people. (We can forget about > changelogs till release candidate status.) Too bad you can't work on it :(. I think it would be nice if you'd put this in hurd-extras. Unfortunately I don't have the time to work on this the first 5 months (perhaps longer). Thanks, Marco _______________________________________________ Bug-hurd mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/bug-hurd
