Fix wrong EOF compare

The read_block reads char one by one until meeting two newline. It's not good for the performance and current code isn't good shape for readability.

This patch enhances speed and clean up.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Randy Dunlap <[email protected]>

Signed-off-by Michal Nazarewicz <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |   34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

Documentation/page_owner.c +13 -21

index 43dde96..96bf481 100644
@@ -28,26 +28,17 @@ static int max_size;
 
 struct block_list *block_head;
 
-int read_block(char *buf, FILE *fin)
+int read_block(char *buf, int buf_size, FILE *fin)
 {
-	int ret = 0;
-	int hit = 0;
-	int val;
-	char *curr = buf;
-
-	for (;;) {
-		val = getc(fin);
-		if (val == EOF) return -1;
-		*curr = val;
-		ret++;
-		if (*curr == '\n' && hit == 1)
-			return ret - 1;
-		else if (*curr == '\n')
-			hit = 1;
-		else
-			hit = 0;
-		curr++;
+	char *curr = buf, *const buf_end = buf + buf_size;
+
+	while (buf_end - curr > 1 && fgets(curr, buf_end - curr, fin)) {
+		if (*curr == '\n') /* empty line */
+			return curr - buf;
+		curr += strlen(curr);
 	}
+
+	return -1; /* EOF or no space left in buf. */
 }
 
 static int compare_txt(struct block_list *l1, struct block_list *l2)

@@ -84,10 +75,12 @@ static void add_list(char *buf, int len)
 	}
 }
 
+#define BUF_SIZE	1024
+
 int main(int argc, char **argv)
 {
 	FILE *fin, *fout;
-	char buf[1024];
+	char buf[BUF_SIZE];
 	int ret, i, count;
 	struct block_list *list2;
 	struct stat st;

@@ -106,11 +99,10 @@ int main(int argc, char **argv)
 	list = malloc(max_size * sizeof(*list));
 
 	for(;;) {
-		ret = read_block(buf, fin);
+		ret = read_block(buf, BUF_SIZE, fin);
 		if (ret < 0)
 			break;
 
-		buf[ret] = '\0';
 		add_list(buf, ret);
 	}
 

Signature · 1 line

1.7.9.5

It occurred to me -- and I know it will sound like a heresy -- that maybe providing an overly long example in C is not the best option here. Why not page_owner.py with the following content instead (not tested):

#!/usr/bin/python
import collections
import sys

counts = collections.defaultdict(int)

txt = ''
for line in sys.stdin:

    if line == '\n':
        counts[txt] += 1
        txt = ''
    else:
        txt += line

counts[txt] += 1

for txt, num in sorted(counts.items(), txt=lambda x: x[1]):
if len(txt) > 1:
print '%d times:\n%s' % num, txt

And it's so a??longa?? only because I chose not to read the whole file at once as in:

counts = collections.defaultdict(int)
for txt in sys.stdin.read().split('\n\n'):

    counts[txt] += 1

Quoted 86 lines

The read_block reads char one by one until meeting two newline. It's not good for the performance and current code isn't good shape for readability.

This patch enhances speed and clean up.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Randy Dunlap <[email protected]>

Signed-off-by Michal Nazarewicz <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |   34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

Documentation/page_owner.c +13 -21

index 43dde96..96bf481 100644
@@ -28,26 +28,17 @@ static int max_size;
 
 struct block_list *block_head;
 
-int read_block(char *buf, FILE *fin)
+int read_block(char *buf, int buf_size, FILE *fin)
 {
-	int ret = 0;
-	int hit = 0;
-	int val;
-	char *curr = buf;
-
-	for (;;) {
-		val = getc(fin);
-		if (val == EOF) return -1;
-		*curr = val;
-		ret++;
-		if (*curr == '\n' && hit == 1)
-			return ret - 1;
-		else if (*curr == '\n')
-			hit = 1;
-		else
-			hit = 0;
-		curr++;
+	char *curr = buf, *const buf_end = buf + buf_size;
+
+	while (buf_end - curr > 1 && fgets(curr, buf_end - curr, fin)) {
+		if (*curr == '\n') /* empty line */
+			return curr - buf;
+		curr += strlen(curr);
 	}
+
+	return -1; /* EOF or no space left in buf. */
 }
 
 static int compare_txt(struct block_list *l1, struct block_list *l2)

@@ -84,10 +75,12 @@ static void add_list(char *buf, int len)
 	}
 }
 
+#define BUF_SIZE	1024
+
 int main(int argc, char **argv)
 {
 	FILE *fin, *fout;
-	char buf[1024];
+	char buf[BUF_SIZE];
 	int ret, i, count;
 	struct block_list *list2;
 	struct stat st;

@@ -106,11 +99,10 @@ int main(int argc, char **argv)
 	list = malloc(max_size * sizeof(*list));
 
 	for(;;) {
-		ret = read_block(buf, fin);
+		ret = read_block(buf, BUF_SIZE, fin);
 		if (ret < 0)
 			break;
 
-		buf[ret] = '\0';
 		add_list(buf, ret);
 	}
 

Signature · 1 line

1.7.9.5

Signature · 4 lines

Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  MichaA? a??mina86a?? Nazarewicz    (o o)
ooo +----<email/xmpp: [email protected]>--------------ooO--(_)--Ooo--

I'm not familar with Python but I can see the point of the program. It's very short and good for maintainace but I have a concern about the size. For working it in embedded side, we have to port python in that machine. :( [...]
In case of that, just small C program when we release product would be good choice.

But I'm not strong aginst on your simple python program. If it is merged, we will just continue to use C program instead of python's one. If you have a strong opinion, send it to akpm as separate patch.

The C standards allows the character type char to be singed or unsinged, depending on the platform and compiler. Most of systems uses signed char, but those based on PowerPC and ARM processors typically use unsigned char. This can lead to unexpected results when the variable is used to compare with EOF(-1). It happens my ARM system and this patch fixes it.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Michal Nazarewicz <[email protected]>

Cc Randy Dunlap <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Documentation/page_owner.c +4 -3

index f0156e1..43dde96 100644
@@ -32,12 +32,13 @@ int read_block(char *buf, FILE *fin)
 {
 	int ret = 0;
 	int hit = 0;
+	int val;
 	char *curr = buf;
 
 	for (;;) {
-		*curr = getc(fin);
-		if (*curr == EOF) return -1;
-
+		val = getc(fin);
+		if (val == EOF) return -1;
+		*curr = val;
 		ret++;
 		if (*curr == '\n' && hit == 1)
 			return ret - 1;

Quoted 1 line · Minchan Kim

The C standards allows the character type char to be singed or

unsinged,

Quoted 1 line

depending on the platform and compiler. Most of systems uses signed

char,

Quoted 1 line

but those based on PowerPC and ARM processors typically use

unsigned char.

Quoted 1 line

This can lead to unexpected results when the variable is used to

compare

Quoted 6 lines

with EOF(-1). It happens my ARM system and this patch fixes it.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Michal Nazarewicz <[email protected]>

Acked-by Michal Nazarewicz <[email protected]>

Quoted 5 lines

Cc Randy Dunlap <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Quoted 14 lines · Michal Nazarewicz

Quoted 10 lines · Minchan Kim

The C standards allows the character type char to be singed or unsinged, depending on the platform and compiler. Most of systems uses signed char, but those based on PowerPC and ARM processors typically use unsigned char. This can lead to unexpected results when the variable is used to compare with EOF(-1). It happens my ARM system and this patch fixes it.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Michal Nazarewicz <[email protected]>

Acked-by Michal Nazarewicz <[email protected]>

Quoted 5 lines

Quoted 5 lines

Cc Randy Dunlap <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

My kernel tree doesn't have Documentation/page_owner.c, where do I find this file?

Quoted 3 lines · Rob Landley

Quoted 2 lines · Michal Nazarewicz

Quoted 1 line · Minchan Kim

The C standards allows the character type char to be singed or

unsinged,

Quoted 1 line

Quoted 1 line

Quoted 1 line

depending on the platform and compiler. Most of systems uses

signed char,

Quoted 1 line

Quoted 1 line

Quoted 1 line

but those based on PowerPC and ARM processors typically use

unsigned char.

Quoted 1 line

Quoted 1 line

Quoted 1 line

This can lead to unexpected results when the variable is used to

compare

Quoted 15 lines

Quoted 14 lines

Quoted 6 lines

with EOF(-1). It happens my ARM system and this patch fixes it.

Cc Mel Gorman <[email protected]>

Cc Andy Whitcroft <[email protected]>

Cc Alexander Nyberg <[email protected]>

Cc Michal Nazarewicz <[email protected]>

Acked-by Michal Nazarewicz <[email protected]>

Quoted 5 lines

Cc Randy Dunlap <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 Documentation/page_owner.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Quoted 1 line

My kernel tree doesn't have Documentation/page_owner.c, where do I

find this file?

It's in -mm (mmotm), so Andrew can/should merge this ...