Re: [PATCH 1/2] Change error messages in ident.c...
- From
Jeff King <peff@peff.net>
- Date
- May 10, 2012, 19:23 UTC
- Message-ID
- <20120510192339.GA32357@sigill.intra.peff.net>
- In-Reply-To
- <1336676770-17965-1-git-send-email-angusgh@gmail.com>
On Thu, May 10, 2012 at 08:06:09PM +0100, Angus Hammond wrote:
> Subject: Re: [PATCH 1/2] Change error messages in ident.c Make error messages > caused by failed reads of the /etc/passwd file easier to understand. > Signed-off-by: Angus Hammond <angusgh@gmail.com>
Holy line-breaks, Batman!
As amusing as I find the existing messages, this is probably a good direction (although I find it unlikely that most people would see the messages under normal use).
I am also tempted to suggest that we simply replace the static buffers with dynamic strbufs. I guess that may open up new vectors for an attacker to convince git to allocate arbitrary amounts of memory, but that is already pretty easy to do, so I doubt it's a big deal.
Show 6 quoted lines
> @@ -46,7 +46,7 @@ static void copy_gecos(const struct passwd *w, char *name, size_t sz)
> if (len < sz)
> name[len] = 0;
> else
> - die("Your parents must have hated you!");
> + die("Your GECOS field is too long.");I know that "GECOS" is the standard name for the field, but I wonder if it is a bit unnecessarily jargon-y. Wouldn't something like:
die("unable to get real name from system password file: name too long");be a little more friendly? It tells what operation we were actually performing, and it doesn't use any jargon.
Show 6 quoted lines
> @@ -106,7 +106,7 @@ static void copy_email(const struct passwd *pw)
> */
> size_t len = strlen(pw->pw_name);
> if (len > sizeof(git_default_email)/2)
> - die("Your sysadmin must hate you!");
> + die("Your name field in is too long.");s/in is/is/. Also, similar complaints to above (if you see this message unexpectedly, you might ask "which name field? One inside a commit object?").
> [...]
And similar comments for the rest of the messages.
-Peff