StringInfo misc. issues
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
Attachments:
StringInfomisc.patchtext/x-patch; name=StringInfomisc.patchDownload
Index: src/backend/lib/stringinfo.c
===================================================================
RCS file: /repositories/edbhome/cvs/EDBAS82/edb/edb-postgres/src/backend/lib/stringinfo.c,v
retrieving revision 1.3
diff -c -r1.3 stringinfo.c
*** src/backend/lib/stringinfo.c 9 Nov 2006 11:09:09 -0000 1.3
--- src/backend/lib/stringinfo.c 29 Aug 2007 14:37:58 -0000
***************
*** 226,232 ****
* bogus data. Without this, we can get an overflow or infinite loop in
* the following.
*/
! if (needed < 0 ||
((Size) needed) >= (MaxAllocSize - (Size) str->len))
elog(ERROR, "invalid string enlargement request size %d",
needed);
--- 226,232 ----
* bogus data. Without this, we can get an overflow or infinite loop in
* the following.
*/
! if (needed <= 0 ||
((Size) needed) >= (MaxAllocSize - (Size) str->len))
elog(ERROR, "invalid string enlargement request size %d",
needed);
***************
*** 259,261 ****
--- 259,272 ----
str->maxlen = newlen;
}
+
+ /*
+ * resetStringInfo
+ * Reset the len field and the data field contents for a fresh start
+ */
+ void
+ resetStringInfo(StringInfo str)
+ {
+ str->len = 0;
+ str->data[0] = '\0';
+ }
Index: src/include/lib/stringinfo.h
===================================================================
RCS file: /repositories/edbhome/cvs/EDBAS82/edb/edb-postgres/src/include/lib/stringinfo.h,v
retrieving revision 1.3
diff -c -r1.3 stringinfo.h
*** src/include/lib/stringinfo.h 9 Nov 2006 11:09:17 -0000 1.3
--- src/include/lib/stringinfo.h 29 Aug 2007 14:37:58 -0000
***************
*** 138,141 ****
--- 138,146 ----
*/
extern void enlargeStringInfo(StringInfo str, int needed);
+ /*------------------------
+ * resetStringInfo
+ * Reset the len field and the data field contents for a fresh start
+ */
+ extern void resetStringInfo(StringInfo str);
#endif /* STRINGINFO_H */
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);
cheers
andrew
cheers
andrew
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)
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
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