Thread

1. StrNCpy

   Maurice Gittens <mgittens@gits.nl> — 1998-03-31T09:12:07Z

   Hi,
   
   While debugging yet another overrun I came across the StrNCpy macro.
   A quick grep of the source tells me that usage of the StrNCpy macro is
   seemingly inconsistent.
   
   Usage 1:
   strptr = palloc(len);    // done is a diffrent context
   ptr = palloc(len + 1);
   StrNCpy(ptr, strptr, len + 1);
   
   Usage 2:
   NameData name;
   StrNCpy(name.data, ptr2name, NAMEDATALEN);
   
   The StrNCpy macro zero terminates the destination buffer.
   
   Usage 1 is gives a read=buffer overrun (which I agree is not the most
   serious of bugs
   if you system doesn't dump core on it).
   Usage 2 makes gives the name a maximum of 31 instead of 32 characters.
   
   Is the maximun name length supposted to be 31 or 32 characters?
   
   With regards from Maurice.
   
   
   

2. Re: [HACKERS] StrNCpy

   Bruce Momjian <maillist@candle.pha.pa.us> — 1998-03-31T15:51:34Z

   > 
   > Hi,
   > 
   > While debugging yet another overrun I came across the StrNCpy macro.
   > A quick grep of the source tells me that usage of the StrNCpy macro is
   > seemingly inconsistent.
   > 
   > Usage 1:
   > strptr = palloc(len);    // done is a diffrent context
   > ptr = palloc(len + 1);
   > StrNCpy(ptr, strptr, len + 1);
   > 
   > Usage 2:
   > NameData name;
   > StrNCpy(name.data, ptr2name, NAMEDATALEN);
   > 
   > The StrNCpy macro zero terminates the destination buffer.
   > 
   > Usage 1 is gives a read=buffer overrun (which I agree is not the most
   > serious of bugs
   > if you system doesn't dump core on it).
   > Usage 2 makes gives the name a maximum of 31 instead of 32 characters.
   > 
   > Is the maximun name length supposted to be 31 or 32 characters?
   
   Thanks for checking into these things for us Maurice.  I zero-terminate
   all name-type fields, so the max is 31, not 32.
   
   The SQL manual page says:
   
          Names  in  SQL  are  sequences  of  less  than NAMEDATALEN
          alphanumeric characters, starting with an alphabetic char-
          acter.   By  default, NAMEDATALEN is set to 32, but at the
          time the system is built, NAMEDATALEN can  be  changed  by
          changing  the  #ifdef  in  src/backend/include/postgres.h.
          Underscore ("_") is considered an alphabetic character.
   
   I updated this manual page when I decided to be consistent and always
   zero-terminate the NAME type.
   
   So the second usage is OK, the first usage:
   	
   	> strptr = palloc(len);    // done is a diffrent context
   	> ptr = palloc(len + 1);
   	> StrNCpy(ptr, strptr, len + 1);
   
   is legally a problem because strNcpy() is doing:
   
       (strncpy((dst),(src),(len)),(len > 0) ? *((dst)+(len)-1)='\0' :(dummyret)NULL,(void)(dst))
   
   and the call to strncpy() is using len, when that is one too large. 
   Now, I know I am going to write over that byte anyway if len >0, so it
   is cleaned up, but it is wrong.  I will change the macro to do:
   
       (strncpy((dst),(src),(len-1)),(len > 0) ? *((dst)+(len)-1)='\0' :(dummyret)NULL,(void)(dst))
   
   or something like that so I check for len == 0.
   
   Here is a patch that does this.  I am applying it now.  Uses the new
   macro formatting style:
   
   ---------------------------------------------------------------------------
   
   *** ./include/c.h.orig	Tue Mar 31 10:42:36 1998
   --- ./include/c.h	Tue Mar 31 10:46:23 1998
   ***************
   *** 703,709 ****
      */
     /* we do this so if the macro is used in an if action, it will work */
     #define StrNCpy(dst,src,len)	\
   ! 	(strncpy((dst),(src),(len)),(len > 0) ? *((dst)+(len)-1)='\0' : (dummyret)NULL,(void)(dst))
     
     /* Get a bit mask of the bits set in non-int32 aligned addresses */
     #define INT_ALIGN_MASK (sizeof(int32) - 1)
   --- 703,717 ----
      */
     /* we do this so if the macro is used in an if action, it will work */
     #define StrNCpy(dst,src,len)	\
   ! ( \
   ! 	((len) > 0) ? \
   ! 	( \
   ! 		strncpy((dst),(src),(len)-1), \
   ! 		*((dst)+(len)-1)='\0' \
   ! 	) \
   ! 	: \
   ! 		(dummyret)NULL,(void)(dst) \
   ! )
     
     /* Get a bit mask of the bits set in non-int32 aligned addresses */
     #define INT_ALIGN_MASK (sizeof(int32) - 1)
   
   -- 
   Bruce Momjian                          |  830 Blythe Avenue
   maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
     +  If your life is a hard drive,     |  (610) 353-9879(w)
     +  Christ can be your backup.        |  (610) 853-3000(h)