[Jfs4bsd-devel] Re: [jfs4bsd] cvs commit: j4b/src/sys/gnu/jfs jfs_imap.c,1.12,1.13

[Sergey L. <de...@as...>]

On Wed, Jun 26, 2002 at 03:39:18PM -0700, Hiten Pandya wrote:
> --- dr...@so... wrote:
> called jfs_misc.c because they are used all over the JFS codebase.  Other
*_subr.c is used as a file with misc functions.
probably it is better to follow the tradition ?


> >  	if (S_ISCHR(ip->i_mode) || S_ISBLK(ip->i_mode))
> > -		dip->di_rdev = cpu_to_le32(kdev_t_to_nr(ip->i_rdev));
> > +		dip->di_rdev = ip->i_rdev;
> 
> Hmm. I am a bit concerned about this bit.  This convert would raise a heck
> lot of endian issues, and secondly, the kdev_t_to_nr() routine is used to
> return the major and minor number of the corresponding device pointed to by
> kdev_t in Linux.  Our functionally equivalent would be dev2unit() which is
> prototyped in /sys/kern/kern_conf.c.
> 
> The correct version should look like:
> dip->di_rdev = cpu_to_le32(dev2unit(ip->i_rdev));

yes, you're absolutely right !
I have a lot of work to be done on jfs_imap.c, anyway.
I had this issue marked in my notebook, so I planned to
fix it. And, sure, that is not the only thing to be fixed there.

> One more thing is important.  It is very important to post some patches on
> the net for review, which make big changes for the only reason that it can
> be reviewed; and we should not just try and get things compiling if they dont
> appear right in the first place... it would just provide us with wrong
> results, hence sucking all our time in debugging with gdb. :-)

yes, that's a good point.

-- 
Sergey Lyubka
Asita Technologies Int, Galway, Ireland