Re: [HACKERS] 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(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)