Skip to contentSkip to conversation

generic strncpy - off-by-one error

PATCH Yoshinori Sato 5 participants 7 messages Aug 12, 2003

lore
Share & tools
lore
#0 lore raw

zero fill count is off-by-one error

string.c renamed from string.c~ +2 -1
@@ -89,7 +89,8 @@
 
 	while (count && (*dest++ = *src++) != '\0')
 		count--;
-	while (count) {
+	count--;
+	while (count > 0) {
 		*dest++ = 0;
 		count--;
 	}
Signature · 2 lines
Yoshinori Sato
<[email protected]>
#1 lore raw
Quoted 1 line · Yoshinori Sato

zero fill count is off-by-one error

I disagree here. With your code, if count becomes 0 within the first while(), you set it to (unsigned)(-1) (because count is size_t), and the second loop will add this number of zeroes after dest (4 billion on 32 bits archs).

The original code seems OK to me.

Cheers,
Willy

Quoted 12 lines
string.c renamed from string.c~ +2 -1
@@ -89,7 +89,8 @@
 
 	while (count && (*dest++ = *src++) != '\0')
 		count--;
-	while (count) {
+	count--;
+	while (count > 0) {
 		*dest++ = 0;
 		count--;
 	}
#2 lore raw
In reply to Yoshinori Sato

I made a mistake. This is a good patch.

string.c renamed from string.c~ +1 -1
@@ -89,7 +89,7 @@
 
 	while (count && (*dest++ = *src++) != '\0')
 		count--;
-	while (count) {
+	while (count > 1) {
 		*dest++ = 0;
 		count--;
 	}
Signature · 2 lines
Yoshinori Sato
<[email protected]>
#3 lore raw

string.c line 92

Quoted 2 lines · Yoshinori Sato
string.c renamed from string.c~ +1 -1
-	while (count) {
+	while (count > 1) {

Given that count is a size_t, which seems to be derived from 'unsigned int' or 'unsigned long' on every platform, how are these any different?

#4 lore raw
Quoted 7 lines · On Tue, 12 Aug 2003 [email protected] wrote:

string.c line 92

Quoted 2 lines · Yoshinori Sato
string.c renamed from string.c~ +1 -1
-	while (count) {
+	while (count > 1) {

Given that count is a size_t, which seems to be derived from 'unsigned int' or 'unsigned long' on every platform, how are these any different?

Er, consider the case of count == 1. Fenceposts can be dangerous things.

Signature · 1 line
Bill Gallafent.
#5 lore raw
Quoted 1 line · William Gallafent

Er, consider the case of count == 1. Fenceposts can be dangerous things.

Amen, given the original code and the first attempt to fix it. I was actually trolling for "accidentally correct" versus "intentionally correct".

/Valdis (who has posted "convince me the code is right THIS time" waay too many times on waaay too many lists already today - including one count-the-on-bits scheme that only worked for an all-zeros input.. ;)

#6 lore raw
In reply to [email protected]

[email protected] writes:

|> On Tue, 12 Aug 2003 23:50:06 +0900, Yoshinori Sato <[email protected]>  said:
|> 
|> > -	while (count) {
|> > +	while (count > 1) {
|> 
|> Given that count is a size_t, which seems to be derived from 'unsigned int'  or
|> 'unsigned long' on every platform, how are these any different?

Let's suppose count == 1.

Andreas.

Signature · 4 lines
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."