Re: [PATCHES] cast pid_t to int when using *printf
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.126r2=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 [EMAIL PROTECTED] | (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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/backend/access/transam/xlog.c === RCS file: /cvsroot/pgsql/src/backend/access/transam/xlog.c,v retrieving revision 1.173 diff -c -c -r1.173 xlog.c *** src/backend/access/transam/xlog.c 12 Oct 2004 21:54:35 - 1.173 --- src/backend/access/transam/xlog.c 14 Oct 2004 19:59:10 - *** *** 1513,1519 * up pre-creating an extra log segment. That seems OK, and better * than holding the lock throughout this lengthy process. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, getpid()); unlink(tmppath); --- 1513,1519 * up pre-creating an extra log segment. That seems OK, and better * than holding the lock throughout this lengthy process. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, (int)getpid()); unlink(tmppath); *** *** 1633,1639 /* * Copy into a temp file name. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, getpid()); unlink(tmppath); --- 1633,1639 /* * Copy into a temp file name. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, (int)getpid()); unlink(tmppath); *** *** 2898,2904 /* * Write into a temp file name. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, getpid()); unlink(tmppath); --- 2898,2904 /* * Write into a temp file name. */ ! snprintf(tmppath, MAXPGPATH, %s/xlogtemp.%d, XLogDir, (int)getpid()); unlink(tmppath); Index: src/backend/postmaster/pgstat.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/pgstat.c,v retrieving revision 1.80 diff -c -c -r1.80 pgstat.c *** src/backend/postmaster/pgstat.c 29 Aug 2004 05:06:46 - 1.80 --- src/backend/postmaster/pgstat.c 14 Oct 2004 19:59:20 - *** *** 1505,1511 snprintf(pgStat_fname, MAXPGPATH, PGSTAT_STAT_FILENAME, DataDir); /* tmpfname need only be set correctly in this process */ snprintf(pgStat_tmpfname, MAXPGPATH, PGSTAT_STAT_TMPFILE, !DataDir, getpid()); /* * Arrange to write the initial status file right away --- 1505,1511 snprintf(pgStat_fname, MAXPGPATH, PGSTAT_STAT_FILENAME, DataDir); /* tmpfname need only be set correctly in this process */ snprintf(pgStat_tmpfname, MAXPGPATH, PGSTAT_STAT_TMPFILE, !DataDir, (int)getpid()); /* * Arrange to write the initial status file right away Index: src/backend/postmaster/postmaster.c === RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v retrieving revision 1.432 diff -c -c -r1.432 postmaster.c *** src/backend/postmaster/postmaster.c 12 Oct 2004 21:54:40 - 1.432 ---
Re: [PATCHES] cast pid_t to int when using *printf
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 longpid_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 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] cast pid_t to int when using *printf
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 [EMAIL PROTECTED] | (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 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] cast pid_t to int when using *printf
Tom Lane wrote: Neil Conway [EMAIL PROTECTED] 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 [EMAIL PROTECTED] | (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 8: explain analyze is your friend
Re: [PATCHES] cast pid_t to int when using *printf
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 longpid_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 [EMAIL PROTECTED] | (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 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] cast pid_t to int when using *printf
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.126r2=1.127 -Neil ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] 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 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] cast pid_t to int when using *printf
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/ ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] cast pid_t to int when using *printf
(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 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] cast pid_t to int when using *printf
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/ ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] cast pid_t to int when using *printf
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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] cast pid_t to int when using *printf
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 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] cast pid_t to int when using *printf
Neil Conway [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
