Re: [PATCH 1/2] Improvate portability: Display pid_t's always as long

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 31, 2008, 07:15 UTC
Message-ID: <7vtzd1bqpt.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <48B9B03F.1080304@gmx.net>

David Soria Parra <dsoria@gmx.net> writes:

Show 7 quoted lines

>> On the other hand, if you are shooting for maximum compatibility perhaps
>> you may want to cast it to "intmax_t" and format as such.
> Yes, good point, casting to long isn't enough. I think it's a good approach to cast the pids to intmax_t as pids
> are also used in git_path() and therefore might result in equal paths for separate processes if
> the pid is higher than int.
>
> so here is an updated patch:

Please wrap lines to reasonable length (e.g. 70-76 cols).

Please move commentary like this that clarifies context of the patch submission to after three-dashes (emulate patches from people with good manners).

> From da5519b3ae5ce84c703aeaab2bc4ea363897c334 Mon Sep 17 00:00:00 2001

Especially, don't paste this line.

> From: David Soria Parra <dsp at>
> Date: Fri, 29 Aug 2008 01:19:43 +0200
> Subject: [PATCH] Improvate portability: Cast pid_t's to intmax_t

"Improvate"?

Including these in your message is not very useful. These in-body headers are used to override what can be read from the real headers of the e-mail message, but you do not have a valid e-mail address here!

Show 12 quoted lines

> Some systems (like e.g. OpenSolaris) define pid_t as long,
> ...
> diff --git a/builtin-commit.c b/builtin-commit.c
> index c870037..90ef3d5 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -320,7 +320,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
>  		die("unable to write new_index file");
>
>  	fd = hold_lock_file_for_update(&false_lock,
> -				       git_path("next-index-%d", getpid()), 1);
> +				       git_path("next-index-%jd", (intmax_t) getpid()), 1);

Some systems we support do not have %j width specifier. I'd suggest casting up to uintmax_t and format with PRIuMAX, which we do define a substitute for portability.