DASH Shell discussions
 help / color / mirror / Atom feed
From: Jilles Tjoelker <jilles@stack.nl>
To: Brian Koropoff <bkoropoff@gmail.com>
Cc: dash@vger.kernel.org
Subject: Re: [PATCH 1/3] Port to Solaris
Date: Fri, 21 Jan 2011 14:06:02 +0100	[thread overview]
Message-ID: <20110121130602.GA70470@stack.nl> (raw)
In-Reply-To: <1295414140.6486.3.camel@gemini>

On Tue, Jan 18, 2011 at 09:15:40PM -0800, Brian Koropoff wrote:
> - Older Solaris does not support %jd (intmax_t) in format
>   strings, but it does support %lld (long long), which is
>   the same size on all architectures it supports.  Do
>   a configure check for the sizes of both and prefer %lld
>   when it is safe to do so.

Pedantically, that's a regression for systems that do support %jd, and I
expect compilers to warn about it. If %jd is available, it must be used
for intmax_t, and not %lld, because that is for long long, not intmax_t.

What you can do is use PRIdMAX from <inttypes.h>, normally defined as
"jd". You can then define this to "lld" or "jd" if it is not defined.
I think this makes the code uglier (just like your change), but oh well.

> - Older Solaris lacks stdint.h, but inttypes.h provides the
>   same types and works on all platforms I've tried dash on,
>   so just use it instead.

<inttypes.h> is defined to be a superset of <stdint.h>, so that is ok.
It is also needed for the PRIdMAX suggestion above.

> [...]
> diff --git a/src/arith_yacc.c b/src/arith_yacc.c
> index 6c5a720..bf21830 100644
> --- a/src/arith_yacc.c
> +++ b/src/arith_yacc.c
> @@ -33,7 +33,6 @@
>   */
>  
>  #include <inttypes.h>
> -#include <stdint.h>
>  #include <stdlib.h>
>  #include "arith_yacc.h"
>  #include "expand.h"

This is useful regardless as the <stdint.h> is redundant. The
<inttypes.h> is already needed here because of imaxdiv().

By the way, I wonder what the advantage of imaxdiv() above separate %
and / is. Compilers can detect the matching between a % b and a / b and
do it in one operation, and any use of imaxdiv() trips gcc's
-Waggregate-return.

-- 
Jilles Tjoelker

  reply	other threads:[~2011-01-21 13:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-19  5:07 Portability patches Brian Koropoff
2011-01-19  5:15 ` [PATCH 1/3] Port to Solaris Brian Koropoff
2011-01-21 13:06   ` Jilles Tjoelker [this message]
2011-03-10 12:16     ` Herbert Xu
2011-03-13  1:18       ` Brian Koropoff
2011-03-13  1:25         ` [PATCH] " Brian Koropoff
2011-03-15  7:36           ` Herbert Xu
2011-01-19  5:15 ` [PATCH 2/3] Port to AIX Brian Koropoff
2011-03-10 12:25   ` Herbert Xu
2011-01-19  5:16 ` [PATCH 3/3] Port to HP-UX Brian Koropoff
2011-03-10 12:26   ` Herbert Xu
2011-03-12 23:54     ` Brian Koropoff
2011-03-15  7:38       ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110121130602.GA70470@stack.nl \
    --to=jilles@stack.nl \
    --cc=bkoropoff@gmail.com \
    --cc=dash@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox