svn commit: r187396 - head/sys/gnu/fs/ext2fs

[Stanislav Sedov]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 19 Jan 2009 19:48:57 +1100 (EST)
Bruce Evans <brde at optusnet.com.au> mentioned:

> > Modified: head/sys/gnu/fs/ext2fs/ext2_vfsops.c
> > ==============================================================================
> > --- head/sys/gnu/fs/ext2fs/ext2_vfsops.c	Sun Jan 18 14:04:56 2009	(r187395)
> > +++ head/sys/gnu/fs/ext2fs/ext2_vfsops.c	Sun Jan 18 14:54:46 2009	(r187396)
> > @@ -5,7 +5,7 @@
> >  *  University of Utah, Department of Computer Science
> >  */
> > /*-
> > - * Copyright (c) 1989, 1991, 1993, 1994
> > + * Copyright (c) 1989, 1991, 1993, 1994
> >  *	The Regents of the University of California.  All rights reserved.
> >  *
> >  * Redistribution and use in source and binary forms, with or without
> > @@ -120,7 +120,7 @@ static int	compute_sb_data(struct vnode
> > static const char *ext2_opts[] = { "from", "export", "acls", "noexec",
> >     "noatime", "union", "suiddir", "multilabel", "nosymfollow",
> >     "noclusterr", "noclusterw", "force", NULL };
> > -
> > +
> 
> The above diff shows null changes, presumably after dropping trailing
> whitespace in the old version.
> 
> Oops, actually the diff is invalid since something dropped the leading
> space in unchanged lines.
>

Yes, seems that svn mail diffs trims the spaces out...
 
> > @@ -318,7 +318,7 @@ static int ext2_check_descriptors (struc
> >         {
> > 		/* examine next descriptor block */
> >                 if ((i % EXT2_DESC_PER_BLOCK(sb)) == 0)
> > -                        gdp = (struct ext2_group_desc *)
> > +                        gdp = (struct ext2_group_desc *)
> > 				sb->s_group_desc[desc_block++]->b_data;
> >                 if (gdp->bg_block_bitmap < block ||
> >                     gdp->bg_block_bitmap >= block + EXT2_BLOCKS_PER_GROUP(sb))
> 
> Here most leading tabs are corrupt in both the new and old versions.  Please
> fix all whitespace on a line if fixing any.
> 

This file has style different from our conventions, so I only stripped
leading spaces and left everything else intact. Should I convert the entire
file to style(9).?

> > @@ -398,19 +398,19 @@ static int compute_sb_data(devvp, es, fs
> >     int logic_sb_block = 1;	/* XXX for now */
> >
> > #if 1
> > -#define V(v)
> > +#define V(v)
> > #else
> > #define V(v)  printf(#v"= %d\n", fs->v);
> > #endif
> 
> The existence of this macro is a bug, since it can only work for one type,
> but fields of different type need to be printed.  Most fields don't have
> type int (the first one printed is s_blocksize which (especially bogusly
> on 64-bit machines) has type u_long, so the non-'#if 1' case would cause
> lots of errors.
> 

In the following commit I casted all fields to (unsigned long) explicitly,
so printfs will work for all fields.

> > @@ -1000,7 +1000,7 @@ ext2_vget(mp, ino, flags, vpp)
> >
> > 	/* Read in the disk contents for the inode, copy into the inode. */
> > #if 0
> > -printf("ext2_vget(%d) dbn= %d ", ino, fsbtodb(fs, ino_to_fsba(fs, ino)));
> > +printf("ext2_vget(%d) dbn= %lu ", ino, fsbtodb(fs, ino_to_fsba(fs, ino)));
> > #endif
> > 	if ((error = bread(ump->um_devvp, fsbtodb(fs, ino_to_fsba(fs, ino)),
> > 	    (int)fs->s_blocksize, NOCRED, &bp)) != 0) {
> 
> ino has type ino_t, so it cannot be printed portably without using a
> cast.  Its actual type is uint32_t.  If this code were actually, this
> would give the following printf format errors on various supported
> machines:
> 
> old: sign mismatch only, since plain int happens to have the same size as
>       ino_t on all supported machines.  gcc doesn't even warn about this
>       error, so this error would be non-fatal
> 
> new: now a size mismatch on machines with 64-bit longs (amd64, ia64, sparc64
>       at least.  gcc warns about this error, so the code is now fatally
>       broken on many supported machines, except it is never actually used.
> 

Actually, the third argument of printf has type of unsigned long for ext2fs
code thanks to arithmetical operations around ino_to_fsba calculations. Thus
the old code has been broken at least for 64-bit platforms, new one should
work everywhere (both for 32 bit an 64bit platforms).

Thanks for comments!
- -- 
Stanislav Sedov
ST4096-RIPE
-----BEGIN PGP SIGNATURE-----

iEYEARECAAYFAkl0SH4ACgkQK/VZk+smlYE7CQCfYtheq6nRYDh/3DWjDJ41hcem
yVMAnjN3sKe8UacMMvH6e1wrR/C/DKES
=84cR
-----END PGP SIGNATURE-----

!DSPAM:4974487d967001758711833!