Re: [Chicken-hackers] [PATCH] Replace unsafe string functions with their safer counterparts

[Peter Bex]

On Thu, Jan 23, 2014 at 12:28:01PM +0100, Christian Kellermann wrote:
> Hi all,
> 
> I propose the following patch. It has been tested on OpenBSD and
> Linux. Should work without troubles on all other OSes as well.
> Please give it a try.

Hi Christian,

After some reconsideration, I think this is probably a good idea.
The big swath of OpenBSD license text for these rather small functions
in our chicken.h really disagreed with me (the license is barely smaller
than the code!), so I looked around a bit for public domain
implementations and found a few.  I decided to use the one by
C.B. Falconer, as it seems to have seen the most review.

I got it from this mirror, as cbfalconer.home.att.net seems down:
http://www.classiccmp.org/cpmarchives/cpm/mirrors/cbfalconer.home.att.net/download/strlcpy.zip

I then ripped out the silly check for a NULL "src" argument, since that's
not allowed by BSD either, and is needless and misguided "convenience"
since we're only including this for compatibility, and we really don't
want to *accidentally* rely on this nonstandard behaviour.  It also
shortens the strlcpy implementation by 3 or so lines.

Finally, to make it work on MingW I had to fix quite a few more things
than I expected, so I present my altered copy containing these fixes for
re-review:

- The C_setenv fallback implementation for when C_GNU_ENV is unset
   (a bit of a bogus check, if you ask me) contained swapped length
   and buffer arguments, causing a segfault upon the first (setenv )
   call I made.  I fixed the swapped calls and moved it to
   posix-common.scm because Windows uses the exact same fallback
   implementation (and C_GNU_ENV is unset in MingW).  The Scheme side
   of these functions is also completely identical, so I moved it
   as well - this seems like a huge change, but it should prevent future
   divergence.
- The Windows implementation of the POSIX unit broke due to the change
   in foundfile()'s "prototype" which is now expected by the Scheme
   code in posix-common, but wasn't carried on to posixwin.scm.
   I moved your version to posix-common.scm as the original macros
   were identical across both platform-specific files.  Same for
   C_opendir, C_closedir and C_readdir.
- In fact, posixwin.scm still contained several calls to C_strcat() and
   C_strcpy() (which you removed from chicken.h), so after fixing the
   compilition, it failed to to link.  I've updated it to use the safe
   versions too.
- You signed off on your own patch :)  I removed this line from the
   commit message so you can sign off again, on my modifications.  That
   way, we'll know that the change which carries both our signoff lines
   is the correct one.

It would be great if people could test this thoroughly on other "fringe"
OSes like AIX, Solaris, Haiku and The Hurd, on the off chance that we
still missed some stuff (that'll probably happen anyway with the release
process, but it would be nice if we can catch it sooner rather than later).

While looking for PD implementations of strlcat I stumbled across the
"Public Domain C library" which, though not offering strl*
implementations, seems very useful if we need to take care of more C
compatibility for broken systems: https://bitbucket.org/pdclib/pdclib

Cheers,
Peter
-- 
http://www.more-magic.net

0001-Replace-unsafe-string-functions-with-their-safer-cou.patch
Description: Text document