Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

Started by Bruce Momjianabout 27 years ago8 messages
#1Bruce Momjian
maillist@candle.pha.pa.us

Update of /usr/local/cvsroot/pgsql/src/backend/lib
In directory hub.org:/tmp/cvs-serv21717

Modified Files:
stringinfo.c
Log Message:
Fix a potential infinite loop in appendStringInfo: would lock
up if first string to be appended to an empty StringInfo was longer
than the initial space allocation.
Also speed it up slightly.

Does this remove the need for vsnprintf?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#2Goran Thyni
goran@bildbasen.se
In reply to: Bruce Momjian (#1)
Overruns (was: 'pgsql/src/backend/lib stringinfo.c')

Bruce Momjian wrote:

Update of /usr/local/cvsroot/pgsql/src/backend/lib
In directory hub.org:/tmp/cvs-serv21717

Modified Files:
stringinfo.c
Log Message:
Fix a potential infinite loop in appendStringInfo: would lock
up if first string to be appended to an empty StringInfo was longer
than the initial space allocation.
Also speed it up slightly.

Does this remove the need for vsnprintf?

I don't think so,
vsprintf is still used if 6 places in to src tree, 5 of them is in
the backend. Each of these should be examined to determent wheater
those can be rewritten or if vsnprintf is needed.

To make matter worse:

guevara-goran# pwd
/usr/local/src/cvs/pgsql/src
guevara-goran# grep -n sprintf `find .` | wc -l
875
guevara-goran# cd backend/
guevara-goran# grep -n sprintf `find .` | wc -l
474

Their is lot of potential overruns in there,
and since pgsql is a net(-able) server we
should take that seriously.

I will look closer at these issues as time permits.

mvh,
--
---------------------------------------------
G�ran Thyni, sysadm, JMS Bildbasen, Kiruna

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: [HACKERS] Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

Bruce Momjian <maillist@candle.pha.pa.us> writes:

Does [stringinfo.c] remove the need for vsnprintf?

Not directly --- the functions it currently provides only know how to
append given strings onto a StringInfo object. There's no formatting
control.

I'm sure we could program around vsnprintf if we were determined
enough, but there isn't any other equally clean way to do what's
needed in tracing. Probably better to spend our effort on providing
a reliable emulation of it for platforms that haven't got it.
(BTW, I have no reason to think that the emulation we have is broken;
I was just objecting to starting to depend on it only a week or so
before a major release.)

Actually, what would be *really* whizzy is some way of sprintf'ing
into a StringInfo, with the ability to auto-expand the StringInfo as
needed. But that requires hooking into the low-level guts of printf,
and AFAIK there's no portable way to do it short of providing your
own complete printf implementation. Not worth it.

regards, tom lane

#4Goran Thyni
goran@bildbasen.se
In reply to: Tom Lane (#3)
Re: [HACKERS] Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

Tom Lane wrote:

Actually, what would be *really* whizzy is some way of sprintf'ing
into a StringInfo, with the ability to auto-expand the StringInfo as
needed. But that requires hooking into the low-level guts of printf,
and AFAIK there's no portable way to do it short of providing your
own complete printf implementation. Not worth it.

Perl does it transparently,
we could embed a perl-engine into the backend. :-)

I am joking but it would be more useful and probably
less work than writing a dynamic sprintf.

But their might be someone out-there on the 'Net that
has done a open-source dynamic sprintf already.

mvh,
--
---------------------------------------------
G�ran Thyni, sysadm, JMS Bildbasen, Kiruna

#5Noname
darcy@druid.net
In reply to: Tom Lane (#3)
Re: [HACKERS] Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

Thus spake Tom Lane

Actually, what would be *really* whizzy is some way of sprintf'ing
into a StringInfo, with the ability to auto-expand the StringInfo as
needed. But that requires hooking into the low-level guts of printf,
and AFAIK there's no portable way to do it short of providing your
own complete printf implementation. Not worth it.

Yah, C really missed the boat there. I remember the Aztec C compiler
for CP/M had this neato function called format() which was used to
implement all the printf functions. You could write your own printf
type function by calling format with the function that handled the
output characters. I was real dissapointed to find out that it wasn't
a standard function.

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.
#6Noname
jwieck@debis.com
In reply to: Noname (#5)
Re: [HACKERS] Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

D'Arcy J.M. Cain wrote:

Yah, C really missed the boat there. I remember the Aztec C compiler
for CP/M had this neato function called format() which was used to

Sniff - I remember the good old DR days of CP/M too. Where
have all the good concepts gone? Someone remembers rasm and
the rsx constructs for CP/M+? They worked before M$ created
that damned, conflicting resident device driver hell.

I still miss pip :-(

Pardon to be totally off-topic here - sneeef.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

#7Noname
darcy@druid.net
In reply to: Noname (#6)
Re: [HACKERS] Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

Thus spake Jan Wieck

Sniff - I remember the good old DR days of CP/M too. Where
have all the good concepts gone? Someone remembers rasm and
the rsx constructs for CP/M+? They worked before M$ created
that damned, conflicting resident device driver hell.

I still miss pip :-(

That's PIP, buddy. Those were the days of CAPS and, by gum, we liked
it that way. :-)

So, you want a copy of my CP/M emulator for Unix?

-- 
D'Arcy J.M. Cain <darcy@{druid|vex}.net>   |  Democracy is three wolves
http://www.druid.net/darcy/                |  and a sheep voting on
+1 416 424 2871     (DoD#0082)    (eNTP)   |  what's for dinner.
#8Bruce Momjian
maillist@candle.pha.pa.us
In reply to: Goran Thyni (#2)
Re: Overruns (was: 'pgsql/src/backend/lib stringinfo.c')

Does this remove the need for vsnprintf?

I don't think so,
vsprintf is still used if 6 places in to src tree, 5 of them is in
the backend. Each of these should be examined to determent wheater
those can be rewritten or if vsnprintf is needed.

To make matter worse:

guevara-goran# pwd
/usr/local/src/cvs/pgsql/src
guevara-goran# grep -n sprintf `find .` | wc -l
875
guevara-goran# cd backend/
guevara-goran# grep -n sprintf `find .` | wc -l
474

Their is lot of potential overruns in there,
and since pgsql is a net(-able) server we
should take that seriously.

I will look closer at these issues as time permits.

Added to TODO:

* fix any sprintf() overruns
* add portable vsnprintf()

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026