From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Marti Raudsepp <marti(at)juffo(dot)org> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Cleanup: Compare pointers to NULL instead of 0 |
Date: | 2010-10-29 19:59:08 |
Message-ID: | 7921.1288382348@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Marti Raudsepp <marti(at)juffo(dot)org> writes:
> Coccinelle found a few places in the code where pointer expressions
> were compared to 0. I have changed them to NULL instead.
Applied, thanks!
> There was one line that I didn't dare to touch, which looks like a
> false positive.
> src/backend/regex/regc_lex.c:849:
> if (v->now - save == 0 || ((int) c > 0 && (int) c <= v->nsubexp))
Hmm. I think this is a false positive in the tool: the difference
between two pointers is an integer, not a pointer, so comparison
to an integer is the semantically correct thing to do --- in fact,
I'd argue that s/0/NULL/ ought to lead to a warning here.
However, by the same token, the code overspecifies the computation.
We don't really need to compute the pointer difference, just compare
the pointers for equality. If it were all integer arithmetic I'd
expect the compiler to figure that out, but given the type issues
maybe not. So I changed it to v->now == save as you suggest. The
only advantage of the previous coding would be if we wanted to change
the test to check for some other number of characters consumed, which
seems mighty unlikely.
> I couldn't find the definition of v (struct vars) anywhere.
It's near the head of regcomp.c.
> But this code doesn't originate from PostgreSQL, so maybe it's not
> worth making cleanups here.
Well, neither is zic, but we still have to look at the warnings.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Leonardo Francalanci | 2010-10-29 20:08:57 | Re: plan time of MASSIVE partitioning ... |
Previous Message | Selena Deckelmann | 2010-10-29 19:50:28 | Tasks for Google Code-In |