Обсуждение: Re: [COMMITTERS] 'pgsql/src/backend/lib stringinfo.c'

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

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

От
Bruce Momjian
Дата:
> 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,
Pennsylvania19026
 


Overruns (was: 'pgsql/src/backend/lib stringinfo.c')

От
Goran Thyni
Дата:
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

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

От
Tom Lane
Дата:
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


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

От
Goran Thyni
Дата:
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


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

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
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.


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

От
jwieck@debis.com (Jan Wieck)
Дата:
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) #

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

От
darcy@druid.net (D'Arcy J.M. Cain)
Дата:
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.


Re: Overruns (was: 'pgsql/src/backend/lib stringinfo.c')

От
Bruce Momjian
Дата:
> > 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,
Pennsylvania19026