Re: [HACKERS] [COMMITTERS] pgsql: Add comments about why errno is set to zero.
Martijn van Oosterhout 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 to zero.
Bruce Momjian 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.
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 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 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.
Tom Lane wrote:
> Bruce Momjian 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 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:
> [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
