Postgres Pro
Downloads
Home   >   mailing lists

Thread: mcxt.c

mcxt.c

From
"Mendola Gaetano"
Date:
A test for null string is missing here:



*** pgsql/src/backend/utils/mmgr/mcxt.c 2003-09-02 13:30:05.000000000 +0200
--- pgsql.old/src/backend/utils/mmgr/mcxt.c 2003-08-04 04:40:08.000000000
+0200
*************** char *
*** 620,632 ****
MemoryContextStrdup(MemoryContext context, const char *string)
{
char *nstr;
-
- if ( !string )
- {
- elog(ERROR, "MemoryContextStrdup called with a NULL pointer");
- return NULL;
- }
-
Size len = strlen(string) + 1;
nstr = (char *) MemoryContextAlloc(context, len);
--- 620,625 ----

Re: mcxt.c

From
Tom Lane
Date:
"Mendola Gaetano" <mendola@bigfoot.com> writes:
> A test for null string is missing here:

> MemoryContextStrdup(MemoryContext context, const char *string)
> {
> char *nstr;
> -
> - if ( !string )
> - {
> - elog(ERROR, "MemoryContextStrdup called with a NULL pointer");
> - return NULL;
> - }

This seems inappropriate to me.  Are you going to suggest that every
routine that takes a pointer parameter needs to explicitly test for
null?  We could bloat the code a great deal that way, and slow it down,
without gaining anything at all in debuggability (IMHO anyway).

If there's a reason for pstrdup in particular to do this, what is it?

            regards, tom lane

Re: mcxt.c

From
"Gaetano Mendola"
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> "Mendola Gaetano" <mendola@bigfoot.com> writes:
> > A test for null string is missing here:
>
> > MemoryContextStrdup(MemoryContext context, const char *string)
> > {
> > char *nstr;
> > -
> > - if ( !string )
> > - {
> > - elog(ERROR, "MemoryContextStrdup called with a NULL pointer");
> > - return NULL;
> > - }
>
> This seems inappropriate to me.  Are you going to suggest that every
> routine that takes a pointer parameter needs to explicitly test for
> null?  We could bloat the code a great deal that way, and slow it down,
> without gaining anything at all in debuggability (IMHO anyway).

Of course I'm not suggesting this, what I'm suggesting is put an
assert( ) if the test can slow down the performances and an "if ( ) "
in places that are not going to touch the performances.

I think that is reasonable.


Regards
Gaetano Mendola

Re: mcxt.c

From
"Gaetano Mendola"
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> "Gaetano Mendola" <mendola@bigfoot.com> writes:
> > "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> >> This seems inappropriate to me.  Are you going to suggest that every
> >> routine that takes a pointer parameter needs to explicitly test for
> >> null?
>
> > Of course I'm not suggesting this, what I'm suggesting is put an
> > assert( ) if the test can slow down the performances and an "if ( ) "
> > in places that are not going to touch the performances.
>
> I see no value at all in an assert.  The code will dump core just fine
> with or without an assert ...

Right but an assert can display information about the file and line number
without debug the application, without mention that reading the code with
the assert is clear what are the precondictions for a call function.



Regards
Gaetano Mendola

Re: mcxt.c

From
Neil Conway
Date:
On Mon, 2003-09-08 at 11:09, Gaetano Mendola wrote:
> "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> > I see no value at all in an assert.  The code will dump core just fine
> > with or without an assert ...
>
> Right but an assert can display information about the file and line number
> without debug the application

I think the percentage of deployments that enable assertions (which
causes a runtime performance hit) but NOT debugging info (which does
not) is pretty small.

ISTM that checking for non-NULL pointers is pretty pointless: just
because a pointer happens to be non-NULL doesn't mean it is any more
valid, and dereferencing a NULL pointer is easy enough to track down in
any case.

-Neil

Re: mcxt.c

From
"Gaetano Mendola"
Date:
"Neil Conway" <neilc@samurai.com> wrote:
> On Mon, 2003-09-08 at 11:09, Gaetano Mendola wrote:
> > "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> > > I see no value at all in an assert.  The code will dump core just fine
> > > with or without an assert ...
> >
> > Right but an assert can display information about the file and line
number
> > without debug the application
>
> I think the percentage of deployments that enable assertions (which
> causes a runtime performance hit) but NOT debugging info (which does
> not) is pretty small.
>
> ISTM that checking for non-NULL pointers is pretty pointless: just
> because a pointer happens to be non-NULL doesn't mean it is any more
> valid, and dereferencing a NULL pointer is easy enough to track down in
> any case.

I'm not speaking only about to test if pointer is null or not but also do
some
assert to better understand wich condition shall be verified in some part of
the
code also to have clear what is going on inside code that may be after years
and years is not clear anymore.

May be I'm not clear enough.
Please tell me when and were an assert shall be used.

Regards
Gaetano Mendola