From: Michael Birk Date: Wed, 10 Apr 1996 22:44:37 -0500 Subject: [PG95]: Bug in textcat() (postgres95-1.01) To report a bug, please complete the following form and send it by email to postgres95@postgres95.vnet.net ============================================================================ POSTGRES95 BUG REPORT TEMPLATE ============================================================================ Your name : Michael Birk Your email address : mbirk@cs.wisc.edu System Configuration - --------------------- Architecture (example: Intel Pentium) : i386 Operating System (example: Linux 1.3.42 ELF) : linux 1.3.28 ELF Postgres95 version (example: Postgres95-1.01) : Postgres95-1.01 Compiler used (example: gcc 2.7.0) : gcc 2.7.2 Please enter a FULL description of your problem: - ------------------------------------------------ The textcat() function was not working properly. Please describe a way to repeat the problem. Please try to provide a concise reproducible example, if at all possible: - ---------------------------------------------------------------------- Any query using textcat() (except the trivial select textcat('a', 'b')) would crash the back end. If you know how this problem might be fixed, list the solution below: - --------------------------------------------------------------------- The textcat() function was not setting the VARSIZE of the varlena structure. This was causing an undefined value to be passed to the palloc function. In addition, the strncat function call is incorrect. The 2nd param to strncat is the maximum number of characters to copy from the src string - _not_ the size of the array. Thus it should be: strncat(VARDATA(result), str2, newlen - VARHDRSZ - strlen(str1)); or, equivalently: strncat(VARDATA(result), str2, strlen(str2)); But then it is obvious that the strncat (as opposed to strcat) is used solely to prevent the terminating null from being copied. This seems dangerous to me - some implementations out there might do this (that is, add the extra null). Why not just use memcpy instead? Then we no longer worry about the extra nulls. In addition, the textout calls aren't necessary. What is needed is a function that determines the actual length of a varlena structure: int textlen (text* t) { int i = 0, max = VARSIZE(t); char *ptr = VARDATA(t); while (i < max && *ptr++) i++; return i; } Then we can summarize textcat as: text* textcat(text* t1, text* t2) { int len1, len2, newlen; text* result; if (t1 == NULL) return t2; if (t2 == NULL) return t1; len1 = textlen (t1); len2 = textlen (t2); newlen = len1 + len2 + VARHDRSZ; result = (text*) palloc (newlen); VARSIZE(result) = newlen; memcpy (VARDATA(result), VARDATA(t1), len1); memcpy (VARDATA(result) + len1, VARDATA(t2), len2); return result; } This is not only more efficient, but much easier to see from a simple reading that it is correct. For your convenience, a patch (against 1.01) is provided: *** src/backend/utils/adt/varlena.c.buggy Wed Apr 10 22:11:52 1996 - --- src/backend/utils/adt/varlena.c Wed Apr 10 22:14:10 1996 *************** *** 198,203 **** - --- 198,220 ---- /* ========== PUBLIC ROUTINES ========== */ + /* + + /* + * textlen - + * returns the actual length of a text* (which may be less than + * the VARSIZE of the text*) + */ + + int textlen (text* t) + { + int i = 0, max = VARSIZE(t); + char *ptr = VARDATA(t); + while (i < max && *ptr++) + i++; + return i; + } + /* * textcat - * takes two text* and returns a text* that is the concatentation of *************** *** 206,233 **** text* textcat(text* t1, text* t2) { ! int newlen; ! char *str1, *str2; text* result; if (t1 == NULL) return t2; if (t2 == NULL) return t1; ! /* since t1, and t2 are non-null, str1 and str2 must also be non-null */ ! str1 = textout(t1); ! str2 = textout(t2); ! /* we use strlen here to calculate the length because the size fields ! of t1, t2 may be longer than necessary to hold the string */ ! newlen = strlen(str1) + strlen(str2) + VARHDRSZ; ! result = (text*)palloc(newlen); ! strcpy(VARDATA(result), str1); ! strncat(VARDATA(result), str2, newlen - VARHDRSZ); ! /* [TRH] Was: ! strcat(VARDATA(result), str2); ! which may corrupt the malloc arena due to writing trailing \0. */ - - pfree(str1); - - pfree(str2); return result; } - --- 223,243 ---- text* textcat(text* t1, text* t2) { ! int len1, len2, newlen; text* result; if (t1 == NULL) return t2; if (t2 == NULL) return t1; ! len1 = textlen (t1); ! len2 = textlen (t2); ! newlen = len1 + len2 + VARHDRSZ; ! result = (text*) palloc (newlen); ! ! VARSIZE(result) = newlen; ! memcpy (VARDATA(result), VARDATA(t1), len1); ! memcpy (VARDATA(result) + len1, VARDATA(t2), len2); return result; } ------------------------------ From: Michael Birk Date: Wed, 10 Apr 1996 23:40:46 -0500 Subject: [PG95]: textcat() bug (followup) This is a followup to my previous bug report on the textcat() bug. The code and patch I sent with that bug report was not correct - - here is the fixed patch (still from the original 1.01 source): *** src/backend/utils/adt/varlena.c.buggy Wed Apr 10 22:11:52 1996 - --- src/backend/utils/adt/varlena.c Wed Apr 10 23:26:21 1996 *************** *** 198,203 **** - --- 198,221 ---- /* ========== PUBLIC ROUTINES ========== */ + /* + + /* + * textlen - + * returns the actual length of a text* (which may be less than + * the VARSIZE of the text*) + */ + + int textlen (text* t) + { + int i = 0; + int max = VARSIZE(t) - VARHDRSZ; + char *ptr = VARDATA(t); + while (i < max && *ptr++) + i++; + return i; + } + /* * textcat - * takes two text* and returns a text* that is the concatentation of *************** *** 206,233 **** text* textcat(text* t1, text* t2) { ! int newlen; ! char *str1, *str2; text* result; if (t1 == NULL) return t2; if (t2 == NULL) return t1; ! /* since t1, and t2 are non-null, str1 and str2 must also be non-null */ ! str1 = textout(t1); ! str2 = textout(t2); ! /* we use strlen here to calculate the length because the size fields ! of t1, t2 may be longer than necessary to hold the string */ ! newlen = strlen(str1) + strlen(str2) + VARHDRSZ; ! result = (text*)palloc(newlen); ! strcpy(VARDATA(result), str1); ! strncat(VARDATA(result), str2, newlen - VARHDRSZ); ! /* [TRH] Was: ! strcat(VARDATA(result), str2); ! which may corrupt the malloc arena due to writing trailing \0. */ - - pfree(str1); - - pfree(str2); return result; } - --- 224,244 ---- text* textcat(text* t1, text* t2) { ! int len1, len2, newlen; text* result; if (t1 == NULL) return t2; if (t2 == NULL) return t1; ! len1 = textlen (t1); ! len2 = textlen (t2); ! newlen = len1 + len2 + VARHDRSZ; ! result = (text*) palloc (newlen); ! ! VARSIZE(result) = newlen; ! memcpy (VARDATA(result), VARDATA(t1), len1); ! memcpy (VARDATA(result) + len1, VARDATA(t2), len2); return result; }