uname check fails on Solaris due to different return value

uname check fails on Solaris due to different return value

[Jakub Kulik]

Hi, I encountered one more minor Solaris failure.

On Solaris, uname syscall returns non-negative value on success and -1 on failure [1], which is different from seemingly all other supported platforms, which return 0 on success (and still -1 on failure). As such, the current check in version.c doesn't work on Solaris.

I managed to fix this with:

--- git-2.56.0/version.c
+++ git-2.56.0/version.c
@@ -83,7 +83,7 @@ int get_uname_info(struct strbuf *buf, u
 {
 	struct utsname uname_info;
 
-	if (uname(&uname_info)) {
+	if (uname(&uname_info) < 0) {
 		strbuf_addf(buf, _("uname() failed with error '%s' (%d)\n"),
 			    strerror(errno),
 			    errno);


which *I think* should work on other platforms as well ([2]), but I might be missing some where this change wouldn't work. Still, I wanted to at least report this.

Thanks and Best regards,
Jakub

[1] https://docs.oracle.com/cd/E88353_01/html/E37841/uname-2.html
    https://smartos.org/man/2/uname
[2] https://pubs.opengroup.org/onlinepubs/009604599/functions/uname.html

Re: uname check fails on Solaris due to different return value

[Junio C Hamano]

Jakub Kulik <jakub.kulik@oracle.com> writes:

> Hi, I encountered one more minor Solaris failure.
>
> On Solaris, uname syscall returns non-negative value on success
> and -1 on failure [1], which is different from seemingly all other
> supported platforms, which return 0 on success (and still -1 on
> failure). As such, the current check in version.c doesn't work on
> Solaris.

It seems that POSIX.1 is pretty clear on this.

    Upon successful completion, a non-negative value shall be
    returned. Otherwise, -1 shall be returned and errno set to
    indicate the error.

Erroring upon seeing a non-zero value like the preimage of your
patch below *is* buggy, and the updated code looks right.

> I managed to fix this with:
>
> --- git-2.56.0/version.c
> +++ git-2.56.0/version.c
> @@ -83,7 +83,7 @@ int get_uname_info(struct strbuf *buf, u
>  {
>  	struct utsname uname_info;
>  
> -	if (uname(&uname_info)) {
> +	if (uname(&uname_info) < 0) {
>  		strbuf_addf(buf, _("uname() failed with error '%s' (%d)\n"),
>  			    strerror(errno),
>  			    errno);
>
>
> which *I think* should work on other platforms as well ([2]), but
> I might be missing some where this change wouldn't work. Still, I
> wanted to at least report this.

Thanks.  Care to make it into a real patch?