> > 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. >> - 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 > -- > 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 > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel >
-- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
