Skip to contentSkip to conversation

fs: Prefer strlcpy() over snprintf()

PATCH In review Jean Delvare 2 participants 3 messages May 9, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fs: Prefer strlcpy() over snprintf()

strlcpy is faster than snprintf when you don't use the returned value.

Cc Joel Becker <[email protected]>
Cc Steven Whitehouse <[email protected]>
Cc Andries Brouwer <[email protected]>

---
A quick grep suggests that there's about 80 more similar cases in the kernel tree which could be fixed.

 fs/configfs/dir.c     |    2 +-
 fs/gfs2/ops_fstype.c  |    4 ++--
 fs/partitions/check.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
fs/configfs/dir.c +1 -1
@@ -1041,7 +1041,7 @@ static int configfs_mkdir(struct inode *
 		goto out_put;
 	}
 
-	snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name);
+	strlcpy(name, dentry->d_name.name, dentry->d_name.len + 1);
 
 	mutex_lock(&subsys->su_mutex);
 	group = NULL;
fs/gfs2/ops_fstype.c +2 -2
@@ -142,8 +142,8 @@ static int init_names(struct gfs2_sbd *s
 	if (!table[0])
 		table = sdp->sd_vfs->s_id;
 
-	snprintf(sdp->sd_proto_name, GFS2_FSNAME_LEN, "%s", proto);
-	snprintf(sdp->sd_table_name, GFS2_FSNAME_LEN, "%s", table);
+	strlcpy(sdp->sd_proto_name, proto, GFS2_FSNAME_LEN);
+	strlcpy(sdp->sd_table_name, table, GFS2_FSNAME_LEN);
 
 	table = sdp->sd_table_name;
 	while ((table = strchr(table, '/')))
fs/partitions/check.c +1 -1
@@ -123,7 +123,7 @@ static int (*check_part[])(struct parsed
 char *disk_name(struct gendisk *hd, int part, char *buf)
 {
 	if (!part)
-		snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
+		strlcpy(buf, hd->disk_name, BDEVNAME_SIZE);
 	else if (isdigit(hd->disk_name[strlen(hd->disk_name)-1]))
 		snprintf(buf, BDEVNAME_SIZE, "%sp%d", hd->disk_name, part);
 	else
Signature · 1 line
Jean Delvare
#1 lore raw

Hi,

The patch looks good, but could you break this into bits? Its easier for me just to put the GFS2 bit through my git tree and the other bits should probably go via another route,

Steve.

Quoted 52 lines · Jean Delvare

strlcpy is faster than snprintf when you don't use the returned value.

Cc Joel Becker <[email protected]>
Cc Steven Whitehouse <[email protected]>
Cc Andries Brouwer <[email protected]>

---
A quick grep suggests that there's about 80 more similar cases in the kernel tree which could be fixed.

 fs/configfs/dir.c     |    2 +-
 fs/gfs2/ops_fstype.c  |    4 ++--
 fs/partitions/check.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
fs/configfs/dir.c +1 -1
@@ -1041,7 +1041,7 @@ static int configfs_mkdir(struct inode *
 		goto out_put;
 	}
 
-	snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name);
+	strlcpy(name, dentry->d_name.name, dentry->d_name.len + 1);
 
 	mutex_lock(&subsys->su_mutex);
 	group = NULL;
fs/gfs2/ops_fstype.c +2 -2
@@ -142,8 +142,8 @@ static int init_names(struct gfs2_sbd *s
 	if (!table[0])
 		table = sdp->sd_vfs->s_id;
 
-	snprintf(sdp->sd_proto_name, GFS2_FSNAME_LEN, "%s", proto);
-	snprintf(sdp->sd_table_name, GFS2_FSNAME_LEN, "%s", table);
+	strlcpy(sdp->sd_proto_name, proto, GFS2_FSNAME_LEN);
+	strlcpy(sdp->sd_table_name, table, GFS2_FSNAME_LEN);
 
 	table = sdp->sd_table_name;
 	while ((table = strchr(table, '/')))
fs/partitions/check.c +1 -1
@@ -123,7 +123,7 @@ static int (*check_part[])(struct parsed
 char *disk_name(struct gendisk *hd, int part, char *buf)
 {
 	if (!part)
-		snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
+		strlcpy(buf, hd->disk_name, BDEVNAME_SIZE);
 	else if (isdigit(hd->disk_name[strlen(hd->disk_name)-1]))
 		snprintf(buf, BDEVNAME_SIZE, "%sp%d", hd->disk_name, part);
 	else
#2 lore raw

Hi Steven,

Quoted 3 lines · Steven Whitehouse

The patch looks good, but could you break this into bits? Its easier for me just to put the GFS2 bit through my git tree and the other bits should probably go via another route,

Thanks for the review. No problem, I'll split the patch and send the individual bits to their respective maintainers now.

Signature · 1 line
Jean Delvare