Skip to content

Commit 07d30af

Browse files
committed
Correct the behavior and return value of snprintf()
According to C99 7.19.6.5, it explicitly defines the behavior and return value of snprintf(). However, the implementation should consider the following description: - If n is zero, nothing is written. - Writes at most n bytes, including the null character. - On success, the return value should be the length of the entire converted string even if n is insufficient to store it. While validating snprintf() in the built-in C library, it was found that the behavior and/or the return value did not match the above description. Therefore, these changes fix the implementation of snprintf() to comply with the specification, and the related test cases are also adjusted to verify the behavior and return value of snprintf().
1 parent ff83f01 commit 07d30af

File tree

6 files changed

+191
-48
lines changed

6 files changed

+191
-48
lines changed

lib/c.c

+117-42
Original file line numberDiff line numberDiff line change
@@ -210,15 +210,83 @@ void __str_base16(char *pb, int val)
210210
}
211211
}
212212

213-
int __format(char *buffer,
214-
int val,
215-
int width,
216-
int zeropad,
217-
int base,
218-
int alternate_form)
213+
/*
214+
* The specification of snprintf() is defined in C99 7.19.6.5,
215+
* and its behavior and return value should comply with the
216+
* following description:
217+
*
218+
* - If n is zero, nothing is written.
219+
* - Writes at most n bytes, including the null character.
220+
* - On success, the return value should be the length of the
221+
* entire converted string even if n is insufficient to store it.
222+
*
223+
* Therefore, the following code defines a structure called fmtbuf_t
224+
* to implement formatted output conversion for the functions in the
225+
* printf() family.
226+
*
227+
* @buf: the current position of the buffer.
228+
* @n : the remaining space of the buffer.
229+
* @len: the number of characters that would have been written
230+
* had n been sufficiently large.
231+
*
232+
* Once a write operation is performed, buf and n will be
233+
* respectively incremented and decremented by the actual written
234+
* size if n is sufficient, and len must be incremented to store
235+
* the length of the entire converted string.
236+
*/
237+
typedef struct {
238+
char *buf;
239+
int n;
240+
int len;
241+
} fmtbuf_t;
242+
243+
void __fmtbuf_write_char(fmtbuf_t *fmtbuf, int val)
244+
{
245+
fmtbuf->len += 1;
246+
247+
/*
248+
* Write the given character when n is greater than 1.
249+
* This means preserving one position for the null character.
250+
*/
251+
if (fmtbuf->n <= 1)
252+
return;
253+
254+
char ch = val & 0xFF;
255+
fmtbuf->buf[0] = ch;
256+
fmtbuf->buf += 1;
257+
fmtbuf->n -= 1;
258+
}
259+
260+
void __fmtbuf_write_str(fmtbuf_t *fmtbuf, char *str, int l)
219261
{
220-
int bi = 0;
221-
char pb[INT_BUF_LEN];
262+
fmtbuf->len += l;
263+
264+
/*
265+
* Write the given string when n is greater than 1.
266+
* This means preserving one position for the null character.
267+
*/
268+
if (fmtbuf->n <= 1)
269+
return;
270+
271+
/*
272+
* If the remaining space is less than the length of the string,
273+
* write only n - 1 bytes.
274+
*/
275+
int sz = fmtbuf->n - 1;
276+
l = l <= sz ? l : sz;
277+
strncpy(fmtbuf->buf, str, l);
278+
fmtbuf->buf += l;
279+
fmtbuf->n -= l;
280+
}
281+
282+
void __format(fmtbuf_t *fmtbuf,
283+
int val,
284+
int width,
285+
int zeropad,
286+
int base,
287+
int alternate_form)
288+
{
289+
char pb[INT_BUF_LEN], ch;
222290
int pbi;
223291

224292
/* set to zeroes */
@@ -249,24 +317,24 @@ int __format(char *buffer,
249317
case 8:
250318
if (alternate_form) {
251319
if (width && zeropad && pb[pbi] != '0') {
252-
buffer[bi++] = '0';
320+
__fmtbuf_write_char(fmtbuf, '0');
253321
width -= 1;
254322
} else if (pb[pbi] != '0')
255323
pb[--pbi] = '0';
256324
}
257325
break;
258326
case 10:
259327
if (width && zeropad && pb[pbi] == '-') {
260-
buffer[bi++] = '-';
328+
__fmtbuf_write_char(fmtbuf, '-');
261329
pbi++;
262330
width--;
263331
}
264332
break;
265333
case 16:
266334
if (alternate_form) {
267335
if (width && zeropad && pb[pbi] != '0') {
268-
buffer[bi++] = '0';
269-
buffer[bi++] = 'x';
336+
__fmtbuf_write_char(fmtbuf, '0');
337+
__fmtbuf_write_char(fmtbuf, 'x');
270338
width -= 2;
271339
} else if (pb[pbi] != '0') {
272340
pb[--pbi] = 'x';
@@ -280,28 +348,22 @@ int __format(char *buffer,
280348
if (width < 0)
281349
width = 0;
282350

351+
ch = zeropad ? '0' : ' ';
283352
while (width) {
284-
buffer[bi++] = zeropad ? '0' : ' ';
353+
__fmtbuf_write_char(fmtbuf, ch);
285354
width--;
286355
}
287356

288-
for (; pbi < INT_BUF_LEN; pbi++)
289-
buffer[bi++] = pb[pbi];
290-
291-
return bi;
357+
__fmtbuf_write_str(fmtbuf, pb + pbi, INT_BUF_LEN - pbi);
292358
}
293359

294-
int __format_to_buf(char *buffer, char *format, int *var_args, int size)
360+
void __format_to_buf(fmtbuf_t *fmtbuf, char *format, int *var_args)
295361
{
296-
int si = 0, bi = 0, pi = 0;
297-
298-
if (size == 0)
299-
return 0;
362+
int si = 0, pi = 0;
300363

301-
while (format[si] && bi < size - 1) {
364+
while (format[si]) {
302365
if (format[si] != '%') {
303-
buffer[bi] = format[si];
304-
bi++;
366+
__fmtbuf_write_char(fmtbuf, format[si]);
305367
si++;
306368
} else {
307369
int w = 0, zp = 0, pp = 0, v = var_args[pi], l;
@@ -328,31 +390,27 @@ int __format_to_buf(char *buffer, char *format, int *var_args, int size)
328390
case 's':
329391
/* append param pi as string */
330392
l = strlen(v);
331-
l = l < size - bi ? l : size - bi;
332-
strncpy(buffer + bi, v, l);
333-
bi += l;
393+
__fmtbuf_write_str(fmtbuf, v, l);
334394
break;
335395
case 'c':
336396
/* append param pi as char */
337-
buffer[bi] = v;
338-
bi += 1;
397+
__fmtbuf_write_char(fmtbuf, v);
339398
break;
340399
case 'o':
341400
/* append param as octal */
342-
bi += __format(buffer + bi, v, w, zp, 8, pp);
401+
__format(fmtbuf, v, w, zp, 8, pp);
343402
break;
344403
case 'd':
345404
/* append param as decimal */
346-
bi += __format(buffer + bi, v, w, zp, 10, 0);
405+
__format(fmtbuf, v, w, zp, 10, 0);
347406
break;
348407
case 'x':
349408
/* append param as hex */
350-
bi += __format(buffer + bi, v, w, zp, 16, pp);
409+
__format(fmtbuf, v, w, zp, 16, pp);
351410
break;
352411
case '%':
353412
/* append literal '%' character */
354-
buffer[bi] = '%';
355-
bi++;
413+
__fmtbuf_write_char(fmtbuf, '%');
356414
si++;
357415
continue;
358416
}
@@ -361,26 +419,43 @@ int __format_to_buf(char *buffer, char *format, int *var_args, int size)
361419
}
362420
}
363421

364-
int len = size - 1 > bi ? bi : size - 1;
365-
buffer[len] = 0;
366-
return len;
422+
/* If n is still greater than 0, set the null character. */
423+
if (fmtbuf->n)
424+
fmtbuf->buf[0] = 0;
367425
}
368426

369427
int printf(char *str, ...)
370428
{
371429
char buffer[200];
372-
int len = __format_to_buf(buffer, str, &str + 4, INT_MAX);
373-
return __syscall(__syscall_write, 1, buffer, len);
430+
fmtbuf_t fmtbuf;
431+
432+
fmtbuf.buf = buffer;
433+
fmtbuf.n = INT_MAX;
434+
fmtbuf.len = 0;
435+
__format_to_buf(&fmtbuf, str, &str + 4);
436+
return __syscall(__syscall_write, 1, buffer, fmtbuf.len);
374437
}
375438

376439
int sprintf(char *buffer, char *str, ...)
377440
{
378-
return __format_to_buf(buffer, str, &str + 4, INT_MAX);
441+
fmtbuf_t fmtbuf;
442+
443+
fmtbuf.buf = buffer;
444+
fmtbuf.n = INT_MAX;
445+
fmtbuf.len = 0;
446+
__format_to_buf(&fmtbuf, str, &str + 4);
447+
return fmtbuf.len;
379448
}
380449

381450
int snprintf(char *buffer, int n, char *str, ...)
382451
{
383-
return __format_to_buf(buffer, str, &str + 4, n);
452+
fmtbuf_t fmtbuf;
453+
454+
fmtbuf.buf = buffer;
455+
fmtbuf.n = n;
456+
fmtbuf.len = 0;
457+
__format_to_buf(&fmtbuf, str, &str + 4);
458+
return fmtbuf.len;
384459
}
385460

386461
int __free_all();

tests/driver.sh

+70-2
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,11 @@ int main() {
16471647
}
16481648
EOF
16491649

1650+
# The following cases validate the behavior and return value of
1651+
# snprintf().
1652+
#
1653+
# This case is a normal case and outputs the complete string
1654+
# because the given buffer size is large enough.
16501655
try_output 16 "Hello World 1123" << EOF
16511656
int main() {
16521657
char buffer[50];
@@ -1656,24 +1661,87 @@ int main() {
16561661
}
16571662
EOF
16581663

1659-
try_output 0 "" << EOF
1664+
# If n is zero, nothing is written.
1665+
#
1666+
# Thus, the output should be the string containing 19 characters
1667+
# for this test case.
1668+
try_output 11 "0000000000000000000" << EOF
16601669
int main() {
16611670
char buffer[20];
1671+
for (int i = 0; i < 19; i++)
1672+
buffer[i] = '0';
1673+
buffer[19] = 0;
16621674
int written = snprintf(buffer, 0, "Number: %d", -37);
16631675
printf("%s", buffer);
16641676
return written;
16651677
}
16661678
EOF
16671679

1668-
try_output 9 "Number: -" << EOF
1680+
# In this case, snprintf() only writes at most 10 bytes (including '\0'),
1681+
# but the return value is 11, which corresponds to the length of
1682+
# "Number: -37".
1683+
try_output 11 "Number: -" << EOF
16691684
int main() {
16701685
char buffer[10];
1686+
for (int i = 0; i < 9; i++)
1687+
buffer[i] = '0';
1688+
buffer[9] = 0;
16711689
int written = snprintf(buffer, 10, "Number: %d", -37);
16721690
printf("%s", buffer);
16731691
return written;
16741692
}
16751693
EOF
16761694

1695+
try_output 14 " 4e 75 6d 62 65 72 3a 20 2d 0 30 30 30 30 30 30 30 30 30 0" << EOF
1696+
int main()
1697+
{
1698+
char buffer[20];
1699+
for (int i = 0; i < 19; i++)
1700+
buffer[i] = '0';
1701+
buffer[19] = 0;
1702+
1703+
int written = snprintf(buffer, 10, "Number: %06d", -35337);
1704+
1705+
for (int i = 0; i < 20; i++)
1706+
printf(" %x", buffer[i]);
1707+
return written;
1708+
}
1709+
EOF
1710+
1711+
# A complex test case for snprintf().
1712+
ans="written = 24
1713+
buffer = buf - 00000
1714+
written = 13
1715+
buffer = aaaa - 0
1716+
written = 19
1717+
buffer = aaaa - 000000777777
1718+
written = 14
1719+
buffer = aaaa - 000000777777
1720+
61 61 61 61 20 2d 20 30 30 30 30 30 30 37 37 37 37 37 37 0 30 30 30 30 30 30 30 30 30 0"
1721+
try_output 0 "$ans" << EOF
1722+
int main()
1723+
{
1724+
char buffer[30];
1725+
for (int i = 0; i < 29; i++)
1726+
buffer[i] = '0';
1727+
buffer[29] = 0;
1728+
1729+
int written = snprintf(buffer, 12, "%s - %018d", "buf", 35133127);
1730+
printf("written = %d\nbuffer = %s\n", written, buffer);
1731+
written = snprintf(buffer, 9, "%s - %#06x", "aaaa", 0xFF);
1732+
printf("written = %d\nbuffer = %s\n", written, buffer);
1733+
written = snprintf(buffer, 30, "%s - %#012o", "aaaa", 0777777);
1734+
printf("written = %d\nbuffer = %s\n", written, buffer);
1735+
written = snprintf(buffer, 0, "%s - %#05x", "bbbbb", 0xAAFF);
1736+
printf("written = %d\nbuffer = %s\n", written, buffer);
1737+
1738+
for (int i = 0; i < 30; i++)
1739+
printf(" %x", buffer[i]);
1740+
printf("\n");
1741+
return 0;
1742+
}
1743+
EOF
1744+
16771745
# test the return value when calling fputc().
16781746
#
16791747
# Since the FILE data type is defined as an int in

tests/snapshots/fib-arm.json

+1-1
Large diffs are not rendered by default.

tests/snapshots/fib-riscv.json

+1-1
Large diffs are not rendered by default.

tests/snapshots/hello-arm.json

+1-1
Large diffs are not rendered by default.

tests/snapshots/hello-riscv.json

+1-1
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)