Re: [PATCH] Rewrite the MAJOR() macro as a call to imajor().

["Robert P. J. Day" <rpjday@mindspring.com>]

On Fri, 4 May 2007, Jan Engelhardt wrote:

> On May 4 2007 04:14, Robert P. J. Day wrote:
> >> On May 3 2007 23:18, Andrew Morton wrote:
> >> >>  	struct inode *i = file->f_mapping->host;
> >> >>
> >> >> -	return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
> >> >> +	return i && S_ISBLK(i->i_mode) && imajor(i) == LOOP_MAJOR;
> >> >>  }
> >> >
> >> >there's no runtime change, and I count a couple hundred MAJORs in the tree.
> >>
> >> Why do we even have imajor() if all it does is calling the MAJOR()
> >> macro?
> >
> >  i'm guessing it's to hide the underlying implementation of
> >extracting the major/minor numbers from an inode, in case that
> >implementation ever changes, which strikes me as perfectly reasonable.
>
> How often has the implementation changed? I think i_rdev has been
> there for a looong time. But yes, doing the MAJOR => imajor
> conversion is preferable. Because you don't need the struct
> declaration for inode then, and may omit to #include <linux/fs.h>.
> (Other things may need fs.h so it's a bit of a corner case.)
>
> >  all i was doing was standardizing the small handful of holdouts.
>
> Please continue, this was not a rant :)

that was it, that one-line patch.  as i pointed out, the entire source
tree currently contains precisely this set of holdouts:

arch/sh/boards/landisk/landisk_pwb.c:   minor = MINOR(inode->i_rdev);
arch/sh/boards/landisk/landisk_pwb.c:   minor = MINOR(inode->i_rdev);
drivers/block/loop.c:   return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
drivers/media/video/ivtv/ivtv-fileops.c:        int minor = MINOR(inode->i_rdev);
include/linux/fs.h:     return MINOR(inode->i_rdev);
include/linux/fs.h:     return MAJOR(inode->i_rdev);
sound/oss/au1550_ac97.c:        int             minor = MINOR(inode->i_rdev);

  the first two are SH-related and i mailed the SH maintainer offline
about them, that previous patch was for the loop.c file, i'm not sure
who's in charge of video/ivtv (hints?), the next two lines are the
header files that define the functions, and i long ago decided not to
invest any time cleaning anything in the OSS directory.

  so, really, it's all of four lines that need to be updated to
complete the job.

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================