Re: cast pid_t to int when using *printf

gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
formatted with %d by a printf-family function. This patch explicitly
casts to int to suppress the warning.

-O

On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote:
> gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
> formatted with %d by a printf-family function.

For curiosity's sake, what formatting escape does gcc prefer?

-Neil

Neil Conway wrote:
> On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote:
>
>>gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
>>formatted with %d by a printf-family function.
>
>
> For curiosity's sake, what formatting escape does gcc prefer?

I don't think there is an escape for pid_t, you always have to cast it.

-O

Am Freitag, 24. September 2004 09:34 schrieb Oliver Jowett:
> Neil Conway wrote:
> > On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote:
> >>gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
> >>formatted with %d by a printf-family function.
> >
> > For curiosity's sake, what formatting escape does gcc prefer?
>
> I don't think there is an escape for pid_t, you always have to cast it.

I think what he was asking is this: Since pid_t has to be a signed integer
type, but gcc does not accept %d for it, then it could be that pid_t is wider
than an int, so casting it to int would potentially lose information.

(Btw., the Windows port defines pid_t as unsigned long; that's surely wrong.)

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:
> Am Freitag, 24. September 2004 09:34 schrieb Oliver Jowett:
>
>>Neil Conway wrote:
>>
>>>On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote:
>>>
>>>>gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
>>>>formatted with %d by a printf-family function.
>>>
>>>For curiosity's sake, what formatting escape does gcc prefer?
>>
>>I don't think there is an escape for pid_t, you always have to cast it.
>
>
> I think what he was asking is this: Since pid_t has to be a signed integer
> type, but gcc does not accept %d for it, then it could be that pid_t is wider
> than an int, so casting it to int would potentially lose information.

pid_t on the Solaris/sparc system is a long (but both int and long are
32 bits). Some experimentation shows that gcc is happy with a %ld format
specifier. But compiling the same code on a Linux/x86 system makes gcc
complain when applying %ld to pid_t, so we will need a cast there either
way.

I notice that elog.c casts MyProcPid to long and uses %ld. Amusingly,
MyProcPid is explicitly an int..

-O

On Fri, 2004-09-24 at 20:31, Oliver Jowett wrote:
> pid_t on the Solaris/sparc system is a long (but both int and long are
> 32 bits). Some experimentation shows that gcc is happy with a %ld format
> specifier. But compiling the same code on a Linux/x86 system makes gcc
> complain when applying %ld to pid_t, so we will need a cast there either
> way.

I guess it would be safest to use %ld and cast pid_t to long. Of course,
this seems a little paranoid -- is there actually a system with
sizeof(pid_t) != 4?

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> I guess it would be safest to use %ld and cast pid_t to long. Of course,
> this seems a little paranoid -- is there actually a system with
> sizeof(pid_t) != 4?

Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
standardize on casting pid_t to int for printing purposes; I think
that's what's being done in more places than not. Also, as you note, we
are using int variables to hold PIDs in many places --- I don't think
it's worth running around and changing those either.

regards, tom lane

Tom Lane wrote:
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > I guess it would be safest to use %ld and cast pid_t to long. Of course,
> > this seems a little paranoid -- is there actually a system with
> > sizeof(pid_t) != 4?
>
> Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
> standardize on casting pid_t to int for printing purposes;

Done.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Oliver Jowett wrote:
> Peter Eisentraut wrote:
> > Am Freitag, 24. September 2004 09:34 schrieb Oliver Jowett:
> >
> >>Neil Conway wrote:
> >>
> >>>On Fri, 2004-09-24 at 16:51, Oliver Jowett wrote:
> >>>
> >>>>gcc (3.2.3 on Solaris 9) warns about a couple of places where a pid_t is
> >>>>formatted with %d by a printf-family function.
> >>>
> >>>For curiosity's sake, what formatting escape does gcc prefer?
> >>
> >>I don't think there is an escape for pid_t, you always have to cast it.
> >
> >
> > I think what he was asking is this: Since pid_t has to be a signed integer
> > type, but gcc does not accept %d for it, then it could be that pid_t is wider
> > than an int, so casting it to int would potentially lose information.
>
> pid_t on the Solaris/sparc system is a long (but both int and long are
> 32 bits). Some experimentation shows that gcc is happy with a %ld format
> specifier. But compiling the same code on a Linux/x86 system makes gcc
> complain when applying %ld to pid_t, so we will need a cast there either
> way.
>
> I notice that elog.c casts MyProcPid to long and uses %ld. Amusingly,
> MyProcPid is explicitly an int..

I see in include/sys/types.h on Solaris 9:

#if defined(_LP64) || defined(_I32LPx)
typedef uint_t nlink_t; /* file link type */
typedef int pid_t; /* process id type */
#else
typedef ulong_t nlink_t; /* (historical version) */
typedef long pid_t; /* (historical version) */
#endif

I am confused why you are seeing long for pid_t? What is your Solaris
system information? If Solaris needs adjustments, there are a lot of
place we print getpid().

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:
> Tom Lane wrote:
>>Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
>>standardize on casting pid_t to int for printing purposes;
>
>
> Done.

Uh, what? Your patch removes the casting of pid_t to int -- Tom was
suggesting that we consistently cast pid_t to int. (Also your patch
removes casting from uid_t to int in the case of geteuid() -- why?)

For instance:

http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.126&r2=1.127

-Neil

Bruce Momjian wrote:

> I see in include/sys/types.h on Solaris 9:
>
> #if defined(_LP64) || defined(_I32LPx)
> typedef uint_t nlink_t; /* file link type */
> typedef int pid_t; /* process id type */
> #else
> typedef ulong_t nlink_t; /* (historical version) */
> typedef long pid_t; /* (historical version) */
> #endif

> I am confused why you are seeing long for pid_t? What is your Solaris
> system information? If Solaris needs adjustments, there are a lot of
> place we print getpid().

This is also what is on the Solaris system I was using. gcc -E showed
that the #else branch was being taken. The #if branch is taken when
compiling in 64-bit mode (gcc -m64).

We're fine so long as everything casts to either int or long. I only saw
warnings from a couple of places that did not do a cast at all.

-O

Neil Conway wrote:
> Bruce Momjian wrote:
> > Tom Lane wrote:
> >>Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
> >>standardize on casting pid_t to int for printing purposes;
> >
> >
> > Done.
>
> Uh, what? Your patch removes the casting of pid_t to int -- Tom was
> suggesting that we consistently cast pid_t to int. (Also your patch
> removes casting from uid_t to int in the case of geteuid() -- why?)
>
> For instance:
>
> http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.126&r2=1.127

>From Tom:

> Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
> standardize on casting pid_t to int for printing purposes;

OK, I read Tom's email saying that we use %d consistently. I didn't
realize he was also saying cast getpid(), but that is easy to do.

Before we had "%ld" sometimes, (int) cast others, and sometimes neither.
It is now consistent and we can make the change all at once.

So I assume everyone wants:

printf("%d", (int) getpid())?

Is this correct?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

OK, 'int' cast added to getpid() calls with %d; patch attached.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Neil Conway wrote:
> > Bruce Momjian wrote:
> > > Tom Lane wrote:
> > >>Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
> > >>standardize on casting pid_t to int for printing purposes;
> > >
> > >
> > > Done.
> >
> > Uh, what? Your patch removes the casting of pid_t to int -- Tom was
> > suggesting that we consistently cast pid_t to int. (Also your patch
> > removes casting from uid_t to int in the case of geteuid() -- why?)
> >
> > For instance:
> >
> > http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/command.c.diff?r1=1.126&r2=1.127
>
> >From Tom:
>
> > Traditionally PIDs fit in 16 bits, let alone 32. I'd recommend that we
> > standardize on casting pid_t to int for printing purposes;
>
> OK, I read Tom's email saying that we use %d consistently. I didn't
> realize he was also saying cast getpid(), but that is easy to do.
>
> Before we had "%ld" sometimes, (int) cast others, and sometimes neither.
> It is now consistent and we can make the change all at once.
>
> So I assume everyone wants:
>
> printf("%d", (int) getpid())?
>
> Is this correct?
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073