Re: [PATCHES] mcxt.c

2003-09-08 Thread Gaetano Mendola 
Tom Lane [EMAIL PROTECTED] wrote:
 Mendola Gaetano [EMAIL PROTECTED] 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

---(end of broadcast)---
TIP 3: 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: [PATCHES] mcxt.c

2003-09-08 Thread Neil Conway 
On Mon, 2003-09-08 at 11:09, Gaetano Mendola wrote:
 Tom Lane [EMAIL PROTECTED] 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



---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faqs/FAQ.html

[PATCHES] mcxt.c

2003-09-07 Thread Mendola Gaetano 
A test for null string is missing here:



*** pgsql/src/backend/utils/mmgr/mcxt.c 2003-09-02 13:30:05.0 +0200
--- pgsql.old/src/backend/utils/mmgr/mcxt.c 2003-08-04 04:40:08.0
+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 



---(end of broadcast)---
TIP 3: 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: [PATCHES] mcxt.c

2003-09-07 Thread Tom Lane 
Mendola Gaetano [EMAIL PROTECTED] 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

---(end of broadcast)---
TIP 9: the planner will ignore your desire to choose an index scan if your
  joining column's datatypes do not match