cast pid_t to int when using *printf

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/

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

In what way is that wrong? A PID on Windows is a DWORD, which is an
unsigned long. Or am I missing something (probably..)?

//Magnus

Am Freitag, 24. September 2004 11:06 schrieb Magnus Hagander:

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

In what way is that wrong? A PID on Windows is a DWORD, which is an
unsigned long. Or am I missing something (probably..)?

The mingw header files define pid_t as int, so we shouldn't redefine it in the
first place. The rest of the POSIX world also assumes that pid_t is signed,
so you might break a bunch of interfaces if it's not. Note that this is
independent of the fact that the actual process identifiers are all positive,
both on Windows and on Unix systems.

(Tertiary note: Never #define one type to another, always use typedef.)

--
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@samurai.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

Peter Eisentraut wrote:

Am Freitag, 24. September 2004 11:06 schrieb Magnus Hagander:

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

In what way is that wrong? A PID on Windows is a DWORD, which is an
unsigned long. Or am I missing something (probably..)?

The mingw header files define pid_t as int, so we shouldn't redefine it in the
first place. The rest of the POSIX world also assumes that pid_t is signed,
so you might break a bunch of interfaces if it's not. Note that this is
independent of the fact that the actual process identifiers are all positive,
both on Windows and on Unix systems.

(Tertiary note: Never #define one type to another, always use typedef.)

I have fixed the Win32 type defines and removed the pid_t because as you
mentioned it was not needed. We only have these left and converted to
typedef:

typedef int uid_t;
typedef int gid_t;
typedef long key_t;

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

Tom Lane wrote:

Neil Conway <neilc@samurai.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@candle.pha.pa.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@candle.pha.pa.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&amp;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&amp;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@candle.pha.pa.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&amp;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@candle.pha.pa.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@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073