Skip to contentSkip to conversation

block: Don't use static to define "void *p" in show_partition_start().

PATCH In review majianpeng 3 participants 4 messages Aug 3, 2012 – Aug 13, 2012 Block layer

lore
Share & tools
lore

majianpeng

author
#0 lore raw

PATCH 1/1 block: Don't use static to define "void *p" in show_partition_start().

I met a odd prblem:read /proc/partitions may return zero.

I wrote a file test.c:
int main()

{
	char buff[4096];
	int ret;
	int fd;
	printf("pid=%d\n",getpid());
	while (1) {
		fd = open("/proc/partitions", O_RDONLY);
		if (fd < 0) {
			printf("open error %s\n", strerror(errno));
			return 0;
		}
		ret = read(fd, buff, 4096);
		if (ret <= 0)
			printf("ret=%d, %s, %ld\n", ret,
				strerror(errno), lseek(fd,0,SEEK_CUR));
		close(fd);
	}
	exit(0);
}

You can reproduce by:
1:while true;do cat /proc/partitions > /dev/null ;done 2:./test

I reviewed the code and found:

Quoted 9 lines
Quoted 9 lines

static void *show_partition_start(struct seq_file *seqf, loff_t *pos)

{
	static void *p;

	p = disk_seqf_start(seqf, pos);
	if (!IS_ERR_OR_NULL(p) && !*pos)
		seq_puts(seqf, "major minor  #blocks  name\n\n");
	return p;
}
		test								cat /proc/partitions
	p = disk_seqf_start()(Not NULL)
									p = disk_seqf_start()(NULL because pos)
	if (!IS_ERR_OR_NULL(p) && !*pos)
Signed-off-by Jianpeng Ma <[email protected]>
---
 block/genhd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/genhd.c +1 -1
index cac7366..d839723 100644
@@ -835,7 +835,7 @@ static void disk_seqf_stop(struct seq_file *seqf, void *v)
 
 static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 {
-	static void *p;
+	void *p;
 
 	p = disk_seqf_start(seqf, pos);
 	if (!IS_ERR_OR_NULL(p) && !*pos)
Signature · 2 lines
1.7.9.5
�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m����鹅�⒏?朓�
maintainer
#1 lore raw
Quoted 53 lines · majianpeng

I met a odd prblem:read /proc/partitions may return zero.

I wrote a file test.c:
int main()

{
	char buff[4096];
	int ret;
	int fd;
	printf("pid=%d\n",getpid());
	while (1) {
		fd = open("/proc/partitions", O_RDONLY);
		if (fd < 0) {
			printf("open error %s\n", strerror(errno));
			return 0;
		}
		ret = read(fd, buff, 4096);
		if (ret <= 0)
			printf("ret=%d, %s, %ld\n", ret,
				strerror(errno), lseek(fd,0,SEEK_CUR));
		close(fd);
	}
	exit(0);
}

You can reproduce by:
1:while true;do cat /proc/partitions > /dev/null ;done 2:./test

I reviewed the code and found:

Quoted 9 lines
Quoted 9 lines

static void *show_partition_start(struct seq_file *seqf, loff_t *pos)

{
	static void *p;

	p = disk_seqf_start(seqf, pos);
	if (!IS_ERR_OR_NULL(p) && !*pos)
		seq_puts(seqf, "major minor  #blocks  name\n\n");
	return p;
}
		test								cat /proc/partitions
	p = disk_seqf_start()(Not NULL)
									p = disk_seqf_start()(NULL because pos)
	if (!IS_ERR_OR_NULL(p) && !*pos)
Signed-off-by Jianpeng Ma <[email protected]>
---
 block/genhd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/genhd.c +0 -0
index cac7366..d839723 100644

block/genhd.c static void disk_seqf_stop(struct seq_file *seqf, void *v) · line 841

Quoted 8 lines
block/genhd.c +1 -1
@@ -835,7 +835,7 @@ static void disk_seqf_stop(struct seq_file *seqf, void *v)
 
 static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 {
-	static void *p;
+	void *p;
 
 	p = disk_seqf_start(seqf, pos);
 	if (!IS_ERR_OR_NULL(p) && !*pos)

Huh, that looks like a clear bug. I've applied it, thanks.

Signature · 1 line
Jens Axboe
#2 lore raw
Quoted 1 line · Jens Axboe

On 08/03/2012 07:07 AM, majianpeng wrote:

[]

Quoted 11 lines
Quoted 11 lines
block/genhd.c +1 -1
index cac7366..d839723 100644
@@ -835,7 +835,7 @@ static void disk_seqf_stop(struct seq_file *seqf, void *v)
 
 static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 {
-	static void *p;
+	void *p;
Quoted 4 lines

block/genhd.c static void disk_seqf_stop(struct seq_file *seqf, void *v) · line 841

Quoted 2 lines
block/genhd.c +0 -0
 	p = disk_seqf_start(seqf, pos);
 	if (!IS_ERR_OR_NULL(p) && !*pos)

Huh, that looks like a clear bug. I've applied it, thanks.

It also looks like a -stable material, don't you think?

Thanks,

/mjt

author
#3 lore raw

On 2012-08-12 23:45 Michael Tokarev <[email protected]> Wrote:

Quoted 19 lines · Jens Axboe
Quoted 1 line · majianpeng

[]

Quoted 11 lines
Quoted 11 lines
block/genhd.c +1 -1
index cac7366..d839723 100644
@@ -835,7 +835,7 @@ static void disk_seqf_stop(struct seq_file *seqf, void *v)
 
 static void *show_partition_start(struct seq_file *seqf, loff_t *pos)
 {
-	static void *p;
+	void *p;
Quoted 4 lines

block/genhd.c static void disk_seqf_stop(struct seq_file *seqf, void *v) · line 841

Quoted 2 lines
block/genhd.c +0 -0
 	p = disk_seqf_start(seqf, pos);
 	if (!IS_ERR_OR_NULL(p) && !*pos)

Huh, that looks like a clear bug. I've applied it, thanks.

Quoted 6 lines

It also looks like a -stable material, don't you think?

Thanks,

/mjt

Yes, all kernel before this patach had this problem and should apply this patch.�翳簕.n�+壏煯壄�+%娝�遍荻w�簕.n�+壏{炳G珴�妠ay�蕠跈�,jf＂穐殢飦�戧鐉_璁(殠娸"濟�m���G珴�⒏?櫒璀�&ｘ忈秈O曟瑉窔v豝�m��� �鹅�⒏?朓�