Re: [LOSTWAGES] Add janitorial task to avoid using strncpy
Mike McCormack wrote:
ChangeLog: * Add janitorial task to avoid using strncpy
------------------------------------------------------------------------
Index: templates/en/janitorial.template =================================================================== RCS file: /home/wine/lostwages/templates/en/janitorial.template,v retrieving revision 1.61 diff -u -r1.61 janitorial.template --- templates/en/janitorial.template 11 Jan 2005 20:33:41 -0000 1.61 +++ templates/en/janitorial.template 13 Jan 2005 15:15:44 -0000 @@ -7,6 +7,24 @@ Janitor's List</a>), and it's high time that we have one too. What is there to clean up? Well, lots of things! :)
+ <h2>Eliminate uses of strncpy/strncpyW</h2> + + <i>strncpy(dst,src,n)</i> has two subtle + <a href="http://weblogs.asp.net/oldnewthing/archive/2005/01/07.aspx"> + problems</a>. The first is that it always fills the whole <i>dst</i> + buffer (<i>n</i> characters). The second is that it doesn't always nul + terminate the <i>dst</i> buffer that it's filling.<p> + + Wine code should avoid the use of strncpy for these reasons, and instead + use lstrcpyA/W or memcpy. You can use the following command to find + occurences of strncpy in the Wine source code:<p> + + <tt>find . -name \*.c -exec grep strncpy {} \; -ls</tt><p> + + The aim is to make sure that we only copy as many bytes as necessary into + the <i>dst</i> buffer, and always nul terminate the buffer when we + intended to.<p> + <h2>Regedit fixes</h2>
Regedit lacks a few features. We need to improve regedit to:
Couldn't we just make a sane implementation of strncpy, not adding more '\0' characters than necessary and making sure the last character is an '\0'? (untested ofcourse ;)) char *strncpy(char *dst,char *src,int n) { int i; for (i = 0; (dst[i] = src[i]) != '\0' && i < n; i++) ; /* n - 1 is '\0'; ensures the end of the string is '\0' */ dst[n - 1] = '\0'; } Now, something like this would at least save us of replacing all strncpyW, wouldn't it? regards, Joris
From: Joris Huizer
Mike McCormack wrote:
ChangeLog: * Add janitorial task to avoid using strncpy
Couldn't we just make a sane implementation of strncpy, not adding more '\0' characters than necessary and making sure the last character is an '\0'?
That sane implementation already exists as lstrcpynA/lstrcpynW in kernel32. Ge van Geldorp.
Ge van Geldorp wrote:
From: Joris Huizer
Mike McCormack wrote:
ChangeLog: * Add janitorial task to avoid using strncpy
Couldn't we just make a sane implementation of strncpy, not adding more '\0' characters than necessary and making sure the last character is an '\0'?
That sane implementation already exists as lstrcpynA/lstrcpynW in kernel32.
Ge van Geldorp.
hmm, well, then what more is needed than s/strncpy\(/lstrcpynA/ and s/strncpyW/lstrcpynW and a note for future code .. ?
Joris Huizer <jorishuizer(a)planet.nl> writes:
hmm, well, then what more is needed than s/strncpy\(/lstrcpynA/ and s/strncpyW/lstrcpynW and a note for future code .. ?
A careful review of the code to ensure that lstrcpyn semantics are really what we want. In several cases we know that we are copying less than the full string, and in that case we have to use memcpy instead, lstrcpyn won't do the right thing. -- Alexandre Julliard julliard(a)winehq.org
On Thu, 13 Jan 2005, Ge van Geldorp wrote: [...]
Couldn't we just make a sane implementation of strncpy, not adding more '\0' characters than necessary and making sure the last character is an '\0'?
That sane implementation already exists as lstrcpynA/lstrcpynW in kernel32.
Btw, Mike actually wrote:
+ Wine code should avoid the use of strncpy for these reasons, and instead + use lstrcpyA/W or memcpy.
So if lstrcpyA/W and not lstrcpynA/W is really what's wanted then we might want to explain a bit more. -- Francois Gouget fgouget(a)free.fr http://fgouget.free.fr/ In a world without fences who needs Gates?
participants (5)
-
Alexandre Julliard -
Francois Gouget -
Ge van Geldorp -
Joris Huizer -
Mike McCormack