Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <xmqq60ciui8o.fsf@gitster.mtv.corp.google.com>
Date: Sun, 17 Sep 2017 12:16:55 +0900
From: Junio C Hamano <gitster@...ox.com>
To: "A. Wilcox" <awilfox@...lielinux.org>
Cc: musl@...ts.openwall.com,  Jeff King <peff@...f.net>,  Kevin Daudt <me@...e.info>,  git@...r.kernel.org
Subject: Re: Git 2.14.1: t6500: error during test on musl libc

"A. Wilcox" <awilfox@...lielinux.org> writes:

>> I did a quick scan for substring "scanf" and read through the 
>> output, and it seems that this is the only one that wants to do
>> the this many characters, e.g. "%42c", conversion.
>> So it seems to me that a real fix has to read the file ourselves
>> and parse up to our HOST_NAME_MAX+1 to see if the hostname refers
>> to us, and fscanf that cannot take "slurp up to this many bytes" is
>> not useful tool to implementing that parsing.
> ...
> Except that is *exactly* *what* *s* *does* (quoting C11 ยง7.21.6.2):
>
> 9   An input item is defined as the longest sequence of input
>     characters which does not exceed any specified field width

Ah, sorry, I completely misread what you meant.

I thought you were suggesting to replace "%<length>c" with just an
unadorned "%s".  You meant that we can use "%<length>s" instead.
And that solution makes sense.  Yes, it is exactly what %<len>s
does.

So something like the following would be a sufficient fix, I guess?

Thanks.

-- >8 --
Subject: gc: call fscanf() with %<len>s, not %<len>c, when reading hostname

Earlier in this codepath, we (ab)used "%<len>c" to read the hostname
recorded in the lockfile into locking_host[HOST_NAME_MAX + 1] while
substituting <len> with the actual value of HOST_NAME_MAX.

This turns out to be incorrect, as it an instruction to read exactly
the specified number of bytes.  We are trying to read at most that
many bytes, we should be using "%<len>s" instead.

Helped-by: A. Wilcox <awilfox@...lielinux.org>
Signed-off-by: Junio C Hamano <gitster@...ox.com>
---
 builtin/gc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 3c78fcb9b1..bb2d6c1fb2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -258,7 +258,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 		int should_exit;
 
 		if (!scan_fmt)
-			scan_fmt = xstrfmt("%s %%%dc", "%"SCNuMAX, HOST_NAME_MAX);
+			scan_fmt = xstrfmt("%s %%%ds", "%"SCNuMAX, HOST_NAME_MAX);
 		fp = fopen(pidfile_path, "r");
 		memset(locking_host, 0, sizeof(locking_host));
 		should_exit =

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.