Thread: StringInfo misc. issues

StringInfo misc. issues

From
NikhilS
Date:
Hi,

I palloc0'ed a variable of type StringInfo and without doing an initStringInfo() (forgot to do it i.e.) tried to append some stuff to it using appendStringInfo(). It went into a tight loop within the function enlargeStringInfo() at:

while (needed > newlen)

Must be a common enough case for a palloc0'ed field right?

The attached patch should fix this.

*** 226,232 ****
!       if (needed < 0 ||
                ((Size) needed) >= (MaxAllocSize - (Size) str->len))
                elog(ERROR, "invalid string enlargement request size %d",
                         needed);
--- 226,232 ----
!       if (needed <= 0 ||
                ((Size) needed) >= (MaxAllocSize - (Size) str->len))
                elog(ERROR, "invalid string enlargement request size %d",
                         needed);

I also found the absence of a function like resetStringInfo() a bit puzzling. A found a lot of places where the code was resetting the "len" field to 0 and assigning '\0' to the data field to reset the variable. This seems to be the only missing API which will be needed while working with the StringInfo type.

Regards,
Nikhils

--
EnterpriseDB               http://www.enterprisedb.com
Attachment

Re: StringInfo misc. issues

From
Andrew Dunstan
Date:

NikhilS wrote:
>
>
> I also found the absence of a function like resetStringInfo() a bit 
> puzzling. A found a lot of places where the code was resetting the 
> "len" field to 0 and assigning '\0' to the data field to reset the 
> variable. This seems to be the only missing API which will be needed 
> while working with the StringInfo type.


er, what? stringinfo.h has:

/*------------------------* resetStringInfo* Clears the current content of the StringInfo, if any. The* StringInfo
remainsvalid.*/
 
extern void resetStringInfo(StringInfo str);


cheers

andrew




cheers

andrew


Re: StringInfo misc. issues

From
Alvaro Herrera
Date:
Andrew Dunstan escribió:
>
>
> NikhilS wrote:
>>
>>
>> I also found the absence of a function like resetStringInfo() a bit 
>> puzzling. A found a lot of places where the code was resetting the "len" 
>> field to 0 and assigning '\0' to the data field to reset the variable. 
>> This seems to be the only missing API which will be needed while working 
>> with the StringInfo type.
>
>
> er, what? stringinfo.h has:
>
> /*------------------------
> * resetStringInfo
> * Clears the current content of the StringInfo, if any. The
> * StringInfo remains valid.
> */
> extern void resetStringInfo(StringInfo str);

I think Neil added this recently.  Maybe NikhilS is looking at 8.2 or
something.

-- 
Alvaro Herrera                          Developer, http://www.PostgreSQL.org/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)


Re: StringInfo misc. issues

From
Tom Lane
Date:
NikhilS <nikkhils@gmail.com> writes:
> The attached patch should fix this.

And break other things, no doubt.  needed = 0 is a perfectly valid
edge case and mustn't be rejected here.  (In fact, I doubt you'd
even get through the regression tests with this patch ... how much
did you test it?)

The real problem with what you describe is that you should have used
makeStringInfo().

> I also found the absence of a function like resetStringInfo() a bit
> puzzling.

CVS HEAD is way ahead of you.
        regards, tom lane


Re: StringInfo misc. issues

From
NikhilS
Date:
Apologies! As Alvaro guessed it correctly I was working with 8.2 sources. Sorry for the noise.

Regards,
Nikhils

On 8/29/07, Tom Lane < tgl@sss.pgh.pa.us> wrote:
NikhilS <nikkhils@gmail.com > writes:
> The attached patch should fix this.

And break other things, no doubt.  needed = 0 is a perfectly valid
edge case and mustn't be rejected here.  (In fact, I doubt you'd
even get through the regression tests with this patch ... how much
did you test it?)

The real problem with what you describe is that you should have used
makeStringInfo().

> I also found the absence of a function like resetStringInfo() a bit
> puzzling.

CVS HEAD is way ahead of you.

                        regards, tom lane



--
EnterpriseDB               http://www.enterprisedb.com