Re: [HACKERS] Warning on contrib/tsearch2

2007-03-29 Thread Teodor Sigaev

code: random backend code should not, not, not be using fopen()
directly.  If you lose control to an elog, which is certainly possible
seeing that this loop calls into the utils/mb subsystem, you'll leak
the file descriptor.  Use AllocateFile/FreeFile instead of fopen/fclose.

Will soon in tsearch_core patch



I find the direct use of malloc/realloc/strdup to be poor style as well
--- backend code that is not using palloc needs to have *very* good
reason to do so, and I see none here.


Already in tsearch_core patch.



--
Teodor Sigaev   E-mail: [EMAIL PROTECTED]
   WWW: http://www.sigaev.ru/

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


[HACKERS] Warning on contrib/tsearch2

2007-03-27 Thread Neil Conway

In CVS HEAD:

contrib/tsearch2/dict_syn.c:124: warning: 'slen' is used uninitialized 
in this function


Induced by the recent pg_verifymbstr() patch.

-Neil


---(end of broadcast)---
TIP 4: Have you searched our list archives?

  http://archives.postgresql.org


Re: [HACKERS] Warning on contrib/tsearch2

2007-03-27 Thread Tom Lane
Neil Conway [EMAIL PROTECTED] writes:
 In CVS HEAD:
 contrib/tsearch2/dict_syn.c:124: warning: 'slen' is used uninitialized 
 in this function
 Induced by the recent pg_verifymbstr() patch.

Seems to be a genuine bug.  Fixed, but I see a worse problem with this
code: random backend code should not, not, not be using fopen()
directly.  If you lose control to an elog, which is certainly possible
seeing that this loop calls into the utils/mb subsystem, you'll leak
the file descriptor.  Use AllocateFile/FreeFile instead of fopen/fclose.

I find the direct use of malloc/realloc/strdup to be poor style as well
--- backend code that is not using palloc needs to have *very* good
reason to do so, and I see none here.

I'm halfway tempted to change postgres.h to #define these functions to
yield errors, and only allow #undef'ing them in the files that are
supposed to access the C library functions directly.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [HACKERS] Warning on contrib/tsearch2

2007-03-27 Thread Andrew Dunstan

Tom Lane wrote:

Neil Conway [EMAIL PROTECTED] writes:
  

In CVS HEAD:
contrib/tsearch2/dict_syn.c:124: warning: 'slen' is used uninitialized 
in this function

Induced by the recent pg_verifymbstr() patch.



Seems to be a genuine bug.  Fixed, but I see a worse problem with this
code: random backend code should not, not, not be using fopen()
directly.  If you lose control to an elog, which is certainly possible
seeing that this loop calls into the utils/mb subsystem, you'll leak
the file descriptor.  Use AllocateFile/FreeFile instead of fopen/fclose.
  


Does that apply to things like plperlu?


I find the direct use of malloc/realloc/strdup to be poor style as well
--- backend code that is not using palloc needs to have *very* good
reason to do so, and I see none here.

I'm halfway tempted to change postgres.h to #define these functions to
yield errors, and only allow #undef'ing them in the files that are
supposed to access the C library functions directly.


  


not a bad idea :-)

cheers

andrew


---(end of broadcast)---
TIP 4: Have you searched our list archives?

  http://archives.postgresql.org


Re: [HACKERS] Warning on contrib/tsearch2

2007-03-27 Thread Tom Lane
Andrew Dunstan [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 ... random backend code should not, not, not be using fopen()
 directly.  If you lose control to an elog, which is certainly possible
 seeing that this loop calls into the utils/mb subsystem, you'll leak
 the file descriptor.  Use AllocateFile/FreeFile instead of fopen/fclose.

 Does that apply to things like plperlu?

For stuff that executes in the regular backend environment, yes.
Anyplace you are calling code that could potentially throw an elog(),
you'd better play by the rules.

I'm prepared to believe that libperl's error handling and resource
management conventions were designed by someone who knew what they were
doing --- so in code called from libperl, you need to follow the libperl
coding rules, instead.  And if you're calling back into the main backend
from a libperl subroutine, you need to provide an impedance match ---
like a subtransaction controlled by a PG_TRY block.

 I'm halfway tempted to change postgres.h to #define these functions to
 yield errors, and only allow #undef'ing them in the files that are
 supposed to access the C library functions directly.

 not a bad idea :-)

Not something to try now, but maybe near the beginning of a devel cycle
...

regards, tom lane

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate