[1/9] test-mergesort: use strbuf_getline()

Commit Message

René Scharfe Oct. 1, 2021, 9:10 a.m. UTC

Strip line ending characters to make sure empty lines are sorted like
sort(1) does.

Signed-off-by: René Scharfe <[e-mail]>
---
 t/helper/test-mergesort.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--
2.33.0

Comments

Ævar Arnfjörð Bjarmason Oct. 2, 2021, 9:08 a.m. UTC | #1

On Fri, Oct 01 2021, René Scharfe wrote:

>  	while (lines) {
> -		printf("%s", lines->text);
> +		puts(lines->text);
>  		lines = lines->next;

Aside: I wonder if we should have a coccicheck for that (not as part of
this series), but maybe it would generate too much noise.

René Scharfe Oct. 2, 2021, 4:56 p.m. UTC | #2

Am 02.10.21 um 11:08 schrieb Ævar Arnfjörð Bjarmason:
>
> On Fri, Oct 01 2021, René Scharfe wrote:
>
>>  	while (lines) {
>> -		printf("%s", lines->text);
>> +		puts(lines->text);
>>  		lines = lines->next;
>
> Aside: I wonder if we should have a coccicheck for that (not as part of
> this series), but maybe it would generate too much noise.

To replace printf("%s\n", s) with puts(s)?  I considered such a semantic
patch before as well.  It would effectively forbid that specific printf
usage.  And why shouldn't it?  puts(3) is easier to use and its call is
shorter.

But puts(3) is also confusing: It adds a trailing newline, but its
sibling fputs(3) doesn't.  And it would look weird in the middle of a
run of printf calls.

Clang already does the replacement with -O1 and GCC even with -O0, so
there is no further performance improvement or object text size
reduction to be gained.

And so I dropped the idea.

René

Patch

diff --git a/t/helper/test-mergesort.c b/t/helper/test-mergesort.c
index c5cffaa4b7..621e2a5197 100644
--- a/t/helper/test-mergesort.c
+++ b/t/helper/test-mergesort.c
@@ -28,9 +28,7 @@  int cmd__mergesort(int argc, const char **argv)
 	struct line *line, *p = NULL, *lines = NULL;
 	struct strbuf sb = STRBUF_INIT;

-	for (;;) {
-		if (strbuf_getwholeline(&sb, stdin, '\n'))
-			break;
+	while (!strbuf_getline(&sb, stdin)) {
 		line = xmalloc(sizeof(struct line));
 		line->text = strbuf_detach(&sb, NULL);
 		if (p) {
@@ -46,7 +44,7 @@  int cmd__mergesort(int argc, const char **argv)
 	lines = llist_mergesort(lines, get_next, set_next, compare_strings);

 	while (lines) {
-		printf("%s", lines->text);
+		puts(lines->text);
 		lines = lines->next;
 	}
 	return 0;