Skip site navigation (1) Skip section navigation (2)

Re: [HACKERS] StrNCpy

From: Bruce Momjian <maillist(at)candle(dot)pha(dot)pa(dot)us>
To: mgittens(at)gits(dot)nl (Maurice Gittens)
Cc: hackers(at)postgreSQL(dot)org
Subject: Re: [HACKERS] StrNCpy
Date: 1998-03-31 15:51:34
Message-ID: 199803311551.KAA07539@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-hackers
> 
> 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(at)candle(dot)pha(dot)pa(dot)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)

In response to

  • StrNCpy at 1998-03-31 09:12:07 from Maurice Gittens

pgsql-hackers by date

Next:From: Maurice GittensDate: 1998-03-31 16:34:30
Subject: Re: [HACKERS] StrNCpy
Previous:From: Bruce MomjianDate: 1998-03-31 15:28:29
Subject: Re: indexing words

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group