Skip to contentSkip to conversation

die kdevname(), die!

PATCH In review Christoph Hellwig 2 participants 4 messages Mar 1, 2003

Subsystems (2)
VFS File locking (flock() and fcntl()/lockf())
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 die kdevname(), die!

Remove the three remaining uses of kdevname():

fs/locks.c:
use MAJOR/MINOR directly as userspace is stupid, this part of the patch is from the maintainer of that piece of code, Matthew Wilcox.
fs/intermezzo/sysctl.c:
printk removed as suggested by you, surrunding code is far from beeing compilable anyway.
fs/partitions/check.c:
replace with __bdevname

and the function plus it's two prototypes in the headers.

fs/libfs.c +0 -11
@@ -332,14 +332,3 @@
 	set_page_dirty(page);
 	return 0;
 }
-
-/*
- * Print device name (in decimal, hexadecimal or symbolic)
- * Note: returns pointer to static data!
- */
-const char * kdevname(kdev_t dev)
-{
-	static char buffer[32];
-	sprintf(buffer, "%02x:%02x", major(dev), minor(dev));
-	return buffer;
-}
fs/locks.c +8 -7
@@ -1785,19 +1785,20 @@
 			       ? (fl->fl_type & F_UNLCK) ? "UNLCK" : "READ "
 			       : (fl->fl_type & F_WRLCK) ? "WRITE" : "READ ");
 	}
-#if WE_CAN_BREAK_LSLK_NOW
 	if (inode) {
+#if WE_CAN_BREAK_LSLK_NOW
 		out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
 				inode->i_sb->s_id, inode->i_ino);
+#else
+		/* userspace relies on this representation of dev_t ;-( */
+		out += sprintf(out, "%d %02x:%02x:%ld ", fl->fl_pid,
+				MAJOR(inode->i_sb->s_dev),
+				MINOR(inode->i_sb->s_dev), inode->i_ino);
+#endif
 	} else {
 		out += sprintf(out, "%d <none>:0 ", fl->fl_pid);
 	}
-#else
-	/* kdevname is a broken interface.  but we expose it to userspace */
-	out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
-			inode ? kdevname(to_kdev_t(inode->i_sb->s_dev)) : "<none>",
-			inode ? inode->i_ino : 0);
-#endif
+
 	if (IS_POSIX(fl)) {
 		if (fl->fl_end == OFFSET_MAX)
 			out += sprintf(out, "%Ld EOF\n", fl->fl_start);
fs/intermezzo/sysctl.c +0 -3
@@ -175,9 +175,6 @@
 		if (errorval < 0) {
 			if (newval == 0)
 				set_device_ro(-errorval, 0);
-			else
-				CERROR("device %s already read only\n",
-				       kdevname(-errorval));
 		} else {
 			if (newval < 0)
 				set_device_ro(-newval, 1);
fs/partitions/check.c +1 -2
@@ -544,7 +544,6 @@
 	}
 
 	dname = kmalloc(sizeof(*dname), GFP_KERNEL);
-
 	if (!dname)
 		return nomem;
 	/*

@@ -558,7 +557,7 @@
 		put_disk(hd);
 	}
 	if (!dname->name) {
-		sprintf(dname->namebuf, "[dev %s]", kdevname(to_kdev_t(dev)));
+		sprintf(dname->namebuf, "[dev %s]", __bdevname(dev));
 		dname->name = dname->namebuf;
 	}
include/linux/fs.h +0 -1
@@ -1069,7 +1069,6 @@
 extern void close_bdev_excl(struct block_device *, int);
 
 extern const char * cdevname(kdev_t);
-extern const char * kdevname(kdev_t);
 extern void init_special_inode(struct inode *, umode_t, dev_t);
 
 /* Invalid inode operations -- fs/bad_inode.c */
include/linux/kdev_t.h +0 -2
@@ -101,8 +101,6 @@
 #define NODEV		(mk_kdev(0,0))
 #define B_FREE		(mk_kdev(0xff,0xff))
 
-extern const char * kdevname(kdev_t);	/* note: returns pointer to static data! */
-
 static inline int kdev_same(kdev_t dev1, kdev_t dev2)
 {
 	return dev1.value == dev2.value;
kernel/ksyms.c +0 -1
@@ -517,7 +517,6 @@
 EXPORT_SYMBOL(vsprintf);
 EXPORT_SYMBOL(vsnprintf);
 EXPORT_SYMBOL(vsscanf);
-EXPORT_SYMBOL(kdevname);
 EXPORT_SYMBOL(__bdevname);
 EXPORT_SYMBOL(cdevname);
 EXPORT_SYMBOL(simple_strtoull);
#1 lore raw

I really think that replacing "kdevname()" with "__bdevname()" is a step in the wrong direction (or at least sideways).

Linus

author
#2 lore raw
Quoted 3 lines · Linus Torvalds

I really think that replacing "kdevname()" with "__bdevname()" is a step in the wrong direction (or at least sideways).

kdevname() is the really wrong interface because it doesn't know whether we have a character or block device. __bdevname is a bit better because it's block-device specific. It's not perfect yet, though.

author
#3 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Linus Torvalds

I really think that replacing "kdevname()" with "__bdevname()" is a step in the wrong direction (or at least sideways).

kdevname() is the really wrong interface because it doesn't know whether we have a character or block device. __bdevname is a bit better because it's block-device specific. It's not perfect yet, though.

And the actual important information is missing in this mail (I'm a bit fast in hitting the send button today :)). The proper replacement for kdevname on blockdevices would be partitions_name - but in this specific case we can't use it because it's the piece of code that constructs the name that is stored for use by partition_name if we don't have any better information.