StrNCpy

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)

Here is a patch that does this. I am applying it now. Uses the new
macro formatting style:

If I cvsup the new sources, wil this mean that I am in sync with you?
BTW is it possible to do a 'cvs update' with cvsup?
I'm sorry if this is a stupid question.

Thanks, with regards from Maurice.

Here is a patch that does this. I am applying it now. Uses the new
macro formatting style:

If I cvsup the new sources, wil this mean that I am in sync with you?

Yes, patch already applied.

BTW is it possible to do a 'cvs update' with cvsup?
I'm sorry if this is a stupid question.

No, cvsup only downloads sources. You need a telnet username/password
from Marc to use cvs for updates.

From src/tools/FAQ_DEV:

There are several ways to obtain the source tree. Occasional developers can
just get the most recent source tree snapshot from ftp.postgresql.org. For
regular developers, you can get CVSup, which is available from
ftp.postgresql.org too. CVSup allows you to download the source tree, then
occasionally update your copy of the source tree with any new changes. Using
CVSup, you don't have to download the entire source each time, only the
changed files. CVSup does not allow developers to update the source tree.

-- 
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)