On Thu, Aug 13, 2009 at 10:12:34PM +0200, Vladimir 'phcoder' Serbinenko wrote: > > > > When you commit this, could you please follow logical order with > > ifdef/else/endif? The negation is less intuitive to read. > Ok > > > >> +#ifdef MODE_UFS2 > >> + grub_uint64_t indir[UFS_BLKSZ (sblock) / sizeof (grub_uint64_t)]; > >> +#else > >> + grub_uint32_t indir[UFS_BLKSZ (sblock) / sizeof (grub_uint32_t)]; > >> +#endif > > > > Can this be made simpler by using typeof() ? (same for the other one below) > > > I tried using sizeof (indir[0]) but it doesn't work since indir isn't > defined at that point.
Ah, of course.. > >> - return (data->ufs_type == UFS1) ? indir[blk] : indir[blk << 1]; > >> + return indir[blk]; > > > > The blk bitshift was accounted for elsewhere? > > > By using correct type (grub_uint64_t vs grub_uint32_t) > > (Btw I assume you've tested > > on both filesystem types). > I tested on FreeBSD variants of UFS1 and UFS2. For Sun UFS1 I relied > on report by Seth Goldberg > >> - ? dirent.namelen_bsd : grub_le_to_cpu16 (dirent.namelen); > >> +#ifdef MODE_UFS2 > >> + namelen = dirent.namelen_bsd; > >> +#else > >> + namelen = grub_le_to_cpu16 (dirent.namelen); > >> +#endif > > > > I wonder if there was a bug here (native endianess assumed for namelen_bsd?) > > > It's one byte Ok. Looks good to me then. Please remember about the ifndef/else/endif thing. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." _______________________________________________ Grub-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/grub-devel
