Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Tom Lane wrote:
[EMAIL PROTECTED] (Bruce Momjian) writes:
Log Message:
---
Add comments about why errno is set to zero.
These comments seem a bit wrongheaded, since checking
LONG_MIN/LONG_MAX is exactly not what we could do to detect an overflow
error.
Yea, I noticed the 0 was listed as another value that needs to be
checked. Should I just change them all to:
errno = 0; /* avoid checking result for failure */
or should I add a macro to c.h as:
/* Sometimes we need to clear errno so we can check errno
* without having to check for a failure value from the function
* call.
*/
#define CLEAR_ERRNO \\
do { \
errno = 0; \\
while (0);
--
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 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Bruce Momjian pgman@candle.pha.pa.us writes:
Should I just change them all to:
errno = 0; /* avoid checking result for failure */
No, that's still a completely inaccurate description of the reason
for having the statement.
or should I add a macro to c.h as:
/* Sometimes we need to clear errno so we can check errno
* without having to check for a failure value from the function
* call.
*/
#define CLEAR_ERRNO \\
do { \
errno = 0; \\
while (0);
I vote neither. Anyone who doesn't understand what this is for will
need to go read the C library man pages for a bit anyway. Nor do I find
CLEAR_ERRNO an improvement over errno = 0.
regards, tom lane
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us writes:
Should I just change them all to:
errno = 0; /* avoid checking result for failure */
No, that's still a completely inaccurate description of the reason
for having the statement.
or should I add a macro to c.h as:
/* Sometimes we need to clear errno so we can check errno
* without having to check for a failure value from the function
* call.
*/
#define CLEAR_ERRNO \\
do { \
errno = 0; \\
while (0);
I vote neither. Anyone who doesn't understand what this is for will
need to go read the C library man pages for a bit anyway. Nor do I find
CLEAR_ERRNO an improvement over errno = 0.
Well, there seems to be enough confusion, even in this email list, that
identifying _why_ errno is being cleared is a good idea.
I modified it to:
errno = 0; /* avoid having to check the result for failure */
--
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 5: don't forget to increase your free space map settings
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Bruce Momjian wrote:
Tom Lane wrote:
or should I add a macro to c.h as:
/* Sometimes we need to clear errno so we can check errno
* without having to check for a failure value from the function
* call.
*/
#define CLEAR_ERRNO \\
do { \
errno = 0; \\
while (0);
May I vote against this kind of use of macros in general? It doesn't
add much value (actually, none in this case) and it makes the code
harder to read. For a pathological example I can point to PHP, which is
so full of strange macros that it's very very hard to read.
Of course there are places where macros are valuable tools, but this
doesn't seem to be one of them.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
On Thu, Dec 01, 2005 at 04:12:30PM -0500, Bruce Momjian wrote: Well, there seems to be enough confusion, even in this email list, that identifying _why_ errno is being cleared is a good idea. I modified it to: errno = 0; /* avoid having to check the result for failure */ I don't know about others but I find that wording ambiguous. Like it's saying that once you've done that it can't fail. I think I'd prefer something like: errno = 0; /* Make error condition detectable */ or even errno = 0; /* clear pending errors */ or errno = 0; /* clear prior detected errors */ YMMV, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a tool for doing 5% of the work and then sitting around waiting for someone else to do the other 95% so you can sue them. pgpnI1LTPZTnb.pgp Description: PGP signature
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set
Martijn van Oosterhout wrote: -- Start of PGP signed section. On Thu, Dec 01, 2005 at 04:12:30PM -0500, Bruce Momjian wrote: Well, there seems to be enough confusion, even in this email list, that identifying _why_ errno is being cleared is a good idea. I modified it to: errno = 0; /* avoid having to check the result for failure */ I don't know about others but I find that wording ambiguous. Like it's saying that once you've done that it can't fail. I think I'd prefer something like: errno = 0; /* Make error condition detectable */ or even errno = 0; /* clear pending errors */ or errno = 0; /* clear prior detected errors */ Maybe it should be: errno = 0; /* Allow unconditional errno check */ -- 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 1: 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: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Bruce Momjian pgman@candle.pha.pa.us writes: I modified it to: errno = 0; /* avoid having to check the result for failure */ Just for the record, that's *still* wrong. It implies that if we tested (result == LONG_MAX errno == ERANGE), without zeroing errno beforehand, the code would be correct. But it would not, because the errno value could still be leftover. The plain fact of the matter is that if you're going to check for strtol overflow at all, you have to zero errno beforehand. This is perfectly well explained in the strtol spec page, and I see no need to duplicate it: Because 0, LONG_MIN and LONG_MAX are returned on error and are also valid returns on success, an application wishing to check for error situations should set errno to 0, then call strtol(), then check errno. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Martijn van Oosterhout kleptog@svana.org writes: errno = 0; /* clear prior detected errors */ That one is at least a correct explanation of what the code is doing... regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set
OK, comments removed, and comment added to port/strtol.c. --- Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: I modified it to: errno = 0; /* avoid having to check the result for failure */ Just for the record, that's *still* wrong. It implies that if we tested (result == LONG_MAX errno == ERANGE), without zeroing errno beforehand, the code would be correct. But it would not, because the errno value could still be leftover. The plain fact of the matter is that if you're going to check for strtol overflow at all, you have to zero errno beforehand. This is perfectly well explained in the strtol spec page, and I see no need to duplicate it: Because 0, LONG_MIN and LONG_MAX are returned on error and are also valid returns on success, an application wishing to check for error situations should set errno to 0, then call strtol(), then check errno. regards, tom lane -- 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 2: Don't 'kill -9' the postmaster
