[twilight-devel] another diff

Mathieu Olivier elric at planetblood.com
Sat Aug 10 03:28:39 EDT 2002


Peter Jay Salzman wrote:
> hola,
> 
> i think it would be better to use strncpy, just to be safe.

Actually, we already use strlcpy/strlcat when necessary, which is even 
better. Or at least we are supposed to do so...  :)

> also, the glib library is pretty standard these days.  it's been stress
> tested by gimp and gtk+.  it's not a large library, but provides (among
> other things) C++ style strings.  strings that can grow and shrink
> depending on their contents.  as long as you stick with their API, this
> eliminates "char *" style buffer overflows.
> 
> if everyone thinks it's a good idea, i can work on replacing char *'s to
> GStrings.  most of the string handling looks like it happens outside of
> actual game play, so it wouldn't slow anything down.

I doubt Knghtbrd would like that.  :)  We try to stay as minimum as 
possible when it comes to required libraries. Also, the glib library is 
pretty standard *on UNIX systems*; I doubt Win32 or MacOSX users have 
it. And what about servers without X-Window? Chances are they won't have 
glib too.
Honestly, using an external lib just for handling strings seems a bad 
idea to me.

> if someone doesn't like that idea, i noticed that most string copying
> happens with strcpy().  i can work on changing them all to strncpy().

strncpy is surely safer than strcpy, but it has its drawbacks too. If 
you really want to clean the code in that regard, please use strlcpy and 
strlcat (in include/strlib.h & src/base/strlib.c)
You can read their man pages on: http://www.openbsd.org/cgi-bin/man.cgi

> --- cvs-twilight-0.1.0/nq/common.c	2001-09-14 23:50:04.000000000 -0700
> +++ my-cvs-twilight-0.1.0/nq/common.c	2002-08-09 15:59:12.000000000 -0700
> @@ -1536,7 +1536,7 @@
>  
>  	dir = Sys_ExpandPath (indir);
>  	Sys_mkdir (dir);
> -	strcpy (com_gamedir, dir);
> +	strncpy (com_gamedir, dir, MAX_OSPATH - 1);
>  

You have forgotten: "com_gamedir[MAX_OSPATH - 1] = '\0'"  after the 
strncpy. Or you can't be sure that com_gamedir will be zero-terminated.
That's why I recommend using strlcpy. "strlcpy (com_gamedir, dir, 
MAX_OSPATH);" is less error-prone, because simpler.

-- 
Mathieu Olivier <elric at planetblood.com>
Co-founder and Lead Programmer of the qBlood Project

The qBlood Project - http://www.planetblood.com/qblood/




More information about the twilight-devel mailing list