On 11/08/2010 10:01 PM, Amos Jeffries wrote:
On 09/11/10 10:44, Alex Rousskov wrote:
On 11/05/2010 06:47 AM, Amos Jeffries wrote:
------------------------------------------------------------
revno: 11017
committer: Amos Jeffries<squ...@treenet.co.nz>
branch nick: trunk
- xstrncpy(prog, EDUI_PROGRAM_NAME, sizeof(prog));
+ xstrncpy(prog, EDUI_PROGRAM_NAME, strlen(EDUI_PROGRAM_NAME));
- xstrncpy(prog, edui_conf.program, sizeof(prog));
+ xstrncpy(prog, edui_conf.program, strlen(edui_conf.program));
- xstrncpy(cbuf, prog, sizeof(cbuf));
+ xstrncpy(cbuf, prog, strlen(prog));
...
These and probably other changes in this commit look like bugs to me:
The third argument to xstrncpy() should be the destination buffer size,
not the source string length. The string length is usually irrelevant
because xstrncpy() will not copy past the 0-terminator of the source
string.
Aw hell. Thought I got rid of those all again. The patch was for
strncat() and bcmp() bugs.
Reverting.
BTW, the following code (which was not changed by this commit), appears
to be buggy or weird and should be changed to use cbuf buffer size, not
cbuf string length:
memset(cbuf, '\0', strlen(cbuf));
Ouch. Thanks.
FWIW, there are more similar memset() calls in that source file as far
as I can see:
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufa, '\0', strlen(bufa));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufa, '\0', strlen(bufa));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufb, '\0', strlen(bufb));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(l->search_filter, '\0', strlen(l->search_filter));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(l->userid, '\0', strlen(l->userid));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufc, '\0', strlen(bufc));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufc, '\0', strlen(bufc));
./helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc:
memset(bufa, '\0', strlen(bufa));
I would not be surprised if these can be rewritten as
*buf = '\0';
./helpers/external_acl/kerberos_ldap_group/kerberos_ldap_group.cc:
memset(optarg, 'X', strlen(optarg));
This last one may not be a bug but should be double checked since it
looks weird.
Cheers,
Alex.
