Обсуждение: [PATCH] Cleanup: Compare pointers to NULL instead of 0

Поиск
Список
Период
Сортировка

[PATCH] Cleanup: Compare pointers to NULL instead of 0

От
Marti Raudsepp
Дата:
Coccinelle found a few places in the code where pointer expressions
were compared to 0. I have changed them to NULL instead.

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))

I couldn't find the definition of v (struct vars) anywhere. Is it
comparing two pointers here? Should it be "v->now == save" instead?

But this code doesn't originate from PostgreSQL, so maybe it's not
worth making cleanups here.

Regards,
Marti

Вложения

Re: [PATCH] Cleanup: Compare pointers to NULL instead of 0

От
Tom Lane
Дата:
Marti Raudsepp <marti@juffo.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


Re: [PATCH] Cleanup: Compare pointers to NULL instead of 0

От
Alvaro Herrera
Дата:
Excerpts from Marti Raudsepp's message of vie oct 29 13:25:03 -0300 2010:
> Coccinelle found a few places in the code where pointer expressions
> were compared to 0. I have changed them to NULL instead.
> 
> 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))
> 
> I couldn't find the definition of v (struct vars) anywhere. Is it
> comparing two pointers here? Should it be "v->now == save" instead?

regcomp.c line 198.  Yes, it's a pointer.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support