Skip to content

Commit 65c3e12

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, consider the following description in that section: * If n is zero, nothing is written. * Output characters beyond the n-1st are discarded rather than being written to the array, and a null character is written at the end of the characters actually written into the array. * The snprintf function returns the number of characters that would have been written had n been sufficiently large. 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 65c3e12

File tree

6 files changed

+194
-48
lines changed

6 files changed

+194
-48
lines changed

lib/c.c

Lines changed: 120 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -210,15 +210,86 @@ 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, and s may be a null pointer.
219+
* - Output characters beyond the n-1st are discarded rather than
220+
* being written to the array.
221+
* - The snprintf function returns the number of characters that would
222+
* have been written had n been sufficiently large, not counting
223+
* the terminating null character, or a negative value if an encoding
224+
* error occurred.
225+
*
226+
* Therefore, the following code defines a structure called fmtbuf_t
227+
* to implement formatted output conversion for the functions in the
228+
* printf() family.
229+
*
230+
* @buf: the current position of buffer.
231+
* @n : the remaining space of the buffer.
232+
* @len: the number of characters that would have been written
233+
* had n been sufficiently large.
234+
*
235+
* Once a write operation is performed, buf and n will be
236+
* respectively incremented and decremented by the actual written
237+
* size if n is sufficient, and len must be incremented to store
238+
* the expected length of the output conversion.
239+
*/
240+
typedef struct {
241+
char *buf;
242+
int n;
243+
int len;
244+
} fmtbuf_t;
245+
246+
void __fmtbuf_write_char(fmtbuf_t *fmtbuf, int val)
247+
{
248+
fmtbuf->len += 1;
249+
250+
/*
251+
* Write the given character when n is greater than 1.
252+
* This means preserving one position for the null character.
253+
*/
254+
if (fmtbuf->n <= 1)
255+
return;
256+
257+
char ch = val & 0xFF;
258+
fmtbuf->buf[0] = ch;
259+
fmtbuf->buf += 1;
260+
fmtbuf->n -= 1;
261+
}
262+
263+
void __fmtbuf_write_str(fmtbuf_t *fmtbuf, char *str, int l)
219264
{
220-
int bi = 0;
221-
char pb[INT_BUF_LEN];
265+
fmtbuf->len += l;
266+
267+
/*
268+
* Write the given string when n is greater than 1.
269+
* This means preserving one position for the null character.
270+
*/
271+
if (fmtbuf->n <= 1)
272+
return;
273+
274+
/*
275+
* If the remaining space is less than the length of the string,
276+
* write only n - 1 bytes.
277+
*/
278+
int sz = fmtbuf->n - 1;
279+
l = l <= sz ? l : sz;
280+
strncpy(fmtbuf->buf, str, l);
281+
fmtbuf->buf += l;
282+
fmtbuf->n -= l;
283+
}
284+
285+
void __format(fmtbuf_t *fmtbuf,
286+
int val,
287+
int width,
288+
int zeropad,
289+
int base,
290+
int alternate_form)
291+
{
292+
char pb[INT_BUF_LEN], ch;
222293
int pbi;
223294

224295
/* set to zeroes */
@@ -249,24 +320,24 @@ int __format(char *buffer,
249320
case 8:
250321
if (alternate_form) {
251322
if (width && zeropad && pb[pbi] != '0') {
252-
buffer[bi++] = '0';
323+
__fmtbuf_write_char(fmtbuf, '0');
253324
width -= 1;
254325
} else if (pb[pbi] != '0')
255326
pb[--pbi] = '0';
256327
}
257328
break;
258329
case 10:
259330
if (width && zeropad && pb[pbi] == '-') {
260-
buffer[bi++] = '-';
331+
__fmtbuf_write_char(fmtbuf, '-');
261332
pbi++;
262333
width--;
263334
}
264335
break;
265336
case 16:
266337
if (alternate_form) {
267338
if (width && zeropad && pb[pbi] != '0') {
268-
buffer[bi++] = '0';
269-
buffer[bi++] = 'x';
339+
__fmtbuf_write_char(fmtbuf, '0');
340+
__fmtbuf_write_char(fmtbuf, 'x');
270341
width -= 2;
271342
} else if (pb[pbi] != '0') {
272343
pb[--pbi] = 'x';
@@ -280,28 +351,22 @@ int __format(char *buffer,
280351
if (width < 0)
281352
width = 0;
282353

354+
ch = zeropad ? '0' : ' ';
283355
while (width) {
284-
buffer[bi++] = zeropad ? '0' : ' ';
356+
__fmtbuf_write_char(fmtbuf, ch);
285357
width--;
286358
}
287359

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

294-
int __format_to_buf(char *buffer, char *format, int *var_args, int size)
363+
void __format_to_buf(fmtbuf_t *fmtbuf, char *format, int *var_args)
295364
{
296-
int si = 0, bi = 0, pi = 0;
297-
298-
if (size == 0)
299-
return 0;
365+
int si = 0, pi = 0;
300366

301-
while (format[si] && bi < size - 1) {
367+
while (format[si]) {
302368
if (format[si] != '%') {
303-
buffer[bi] = format[si];
304-
bi++;
369+
__fmtbuf_write_char(fmtbuf, format[si]);
305370
si++;
306371
} else {
307372
int w = 0, zp = 0, pp = 0, v = var_args[pi], l;
@@ -328,31 +393,27 @@ int __format_to_buf(char *buffer, char *format, int *var_args, int size)
328393
case 's':
329394
/* append param pi as string */
330395
l = strlen(v);
331-
l = l < size - bi ? l : size - bi;
332-
strncpy(buffer + bi, v, l);
333-
bi += l;
396+
__fmtbuf_write_str(fmtbuf, v, l);
334397
break;
335398
case 'c':
336399
/* append param pi as char */
337-
buffer[bi] = v;
338-
bi += 1;
400+
__fmtbuf_write_char(fmtbuf, v);
339401
break;
340402
case 'o':
341403
/* append param as octal */
342-
bi += __format(buffer + bi, v, w, zp, 8, pp);
404+
__format(fmtbuf, v, w, zp, 8, pp);
343405
break;
344406
case 'd':
345407
/* append param as decimal */
346-
bi += __format(buffer + bi, v, w, zp, 10, 0);
408+
__format(fmtbuf, v, w, zp, 10, 0);
347409
break;
348410
case 'x':
349411
/* append param as hex */
350-
bi += __format(buffer + bi, v, w, zp, 16, pp);
412+
__format(fmtbuf, v, w, zp, 16, pp);
351413
break;
352414
case '%':
353415
/* append literal '%' character */
354-
buffer[bi] = '%';
355-
bi++;
416+
__fmtbuf_write_char(fmtbuf, '%');
356417
si++;
357418
continue;
358419
}
@@ -361,26 +422,43 @@ int __format_to_buf(char *buffer, char *format, int *var_args, int size)
361422
}
362423
}
363424

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

369430
int printf(char *str, ...)
370431
{
371432
char buffer[200];
372-
int len = __format_to_buf(buffer, str, &str + 4, INT_MAX);
373-
return __syscall(__syscall_write, 1, buffer, len);
433+
fmtbuf_t fmtbuf;
434+
435+
fmtbuf.buf = buffer;
436+
fmtbuf.n = INT_MAX;
437+
fmtbuf.len = 0;
438+
__format_to_buf(&fmtbuf, str, &str + 4);
439+
return __syscall(__syscall_write, 1, buffer, fmtbuf.len);
374440
}
375441

376442
int sprintf(char *buffer, char *str, ...)
377443
{
378-
return __format_to_buf(buffer, str, &str + 4, INT_MAX);
444+
fmtbuf_t fmtbuf;
445+
446+
fmtbuf.buf = buffer;
447+
fmtbuf.n = INT_MAX;
448+
fmtbuf.len = 0;
449+
__format_to_buf(&fmtbuf, str, &str + 4);
450+
return fmtbuf.len;
379451
}
380452

381453
int snprintf(char *buffer, int n, char *str, ...)
382454
{
383-
return __format_to_buf(buffer, str, &str + 4, n);
455+
fmtbuf_t fmtbuf;
456+
457+
fmtbuf.buf = buffer;
458+
fmtbuf.n = n;
459+
fmtbuf.len = 0;
460+
__format_to_buf(&fmtbuf, str, &str + 4);
461+
return fmtbuf.len;
384462
}
385463

386464
int __free_all();

tests/driver.sh

Lines changed: 70 additions & 2 deletions
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

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

tests/snapshots/fib-riscv.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

tests/snapshots/hello-arm.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

tests/snapshots/hello-riscv.json

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)