Skip to content

Commit 5b156fc

Browse files
pabigotpabigot
committed
lib: cbprintf: refactor for upcoming enhancement
The extraction of values from varargs was inlined in cbvprintf() because va_list arguments cannot be passed into subroutines by reference, and there was only one place that needed this functionality. An upcoming enhancement requires extracting the arguments but processing them later, so outline the extraction code. Signed-off-by: Peter Bigot <[email protected]>
1 parent 7f98034 commit 5b156fc

File tree

1 file changed

+196
-160
lines changed

1 file changed

+196
-160
lines changed

‎lib/os/cbprintf_complete.c‎

Lines changed: 196 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,38 @@ struct conversion {
266266
};
267267
};
268268

269+
/* State used for processing format conversions with values taken from
270+
* varargs.
271+
*/
272+
struct cbprintf_state {
273+
/* A copy of the captured arguments on the call stack.
274+
*
275+
* See https://stackoverflow.com/a/8048892 for why we can't
276+
* just pass a pointer to the passed ap argument.
277+
*/
278+
va_list ap;
279+
280+
/* A conversion specifier extracted with
281+
* extract_conversion()
282+
*/
283+
struct conversion conv;
284+
285+
/* The value from ap extracted with data from conv. */
286+
union argument_value value;
287+
288+
/* The width to use when emitting the value.
289+
*
290+
* Negative values indicate this is not used.
291+
*/
292+
int width;
293+
294+
/* The precision to use when emitting the value.
295+
*
296+
* Negative values indicate this is not used.
297+
*/
298+
int precision;
299+
};
300+
269301
/** Get a size represented as a sequence of decimal digits.
270302
*
271303
* @param[inout] str where to read from. Updated to point to the first
@@ -1278,10 +1310,167 @@ static int outs(cbprintf_cb out,
12781310
return (int)count;
12791311
}
12801312

1313+
/* Pull from the stack all information related to the conversion
1314+
* specified in state.
1315+
*/
1316+
static void pull_va_args(struct cbprintf_state *state)
1317+
{
1318+
struct conversion *const conv = &state->conv;
1319+
union argument_value *const value = &state->value;
1320+
1321+
state->width = -1;
1322+
state->precision = -1;
1323+
1324+
/* If dynamic width is specified, process it, otherwise set
1325+
* with if present.
1326+
*/
1327+
if (conv->width_star) {
1328+
state->width = va_arg(state->ap, int);
1329+
1330+
if (state->width < 0) {
1331+
conv->flag_dash = true;
1332+
state->width = -state->width;
1333+
}
1334+
} else if (conv->width_present) {
1335+
state->width = conv->width_value;
1336+
}
1337+
1338+
/* If dynamic precision is specified, process it, otherwise
1339+
* set precision if present. For floating point where
1340+
* precision is not present use 6.
1341+
*/
1342+
if (conv->prec_star) {
1343+
int arg = va_arg(state->ap, int);
1344+
1345+
if (arg < 0) {
1346+
conv->prec_present = false;
1347+
} else {
1348+
state->precision = arg;
1349+
}
1350+
} else if (conv->prec_present) {
1351+
state->precision = conv->prec_value;
1352+
}
1353+
1354+
/* Reuse width and precision memory in conv for value
1355+
* padding counts.
1356+
*/
1357+
conv->pad0_value = 0;
1358+
conv->pad0_pre_exp = 0;
1359+
1360+
/* FP conversion requires knowing the precision. */
1361+
if (IS_ENABLED(CONFIG_CBPRINTF_FP_SUPPORT)
1362+
&& (conv->specifier_cat == SPECIFIER_FP)
1363+
&& !conv->prec_present) {
1364+
if (conv->specifier_a) {
1365+
state->precision = FRACTION_HEX;
1366+
} else {
1367+
state->precision = 6;
1368+
}
1369+
}
1370+
1371+
enum specifier_cat_enum specifier_cat
1372+
= (enum specifier_cat_enum)conv->specifier_cat;
1373+
enum length_mod_enum length_mod
1374+
= (enum length_mod_enum)conv->length_mod;
1375+
1376+
/* Extract the value based on the argument category and length.
1377+
*
1378+
* Note that the length modifier doesn't affect the value of a
1379+
* pointer argument.
1380+
*/
1381+
if (specifier_cat == SPECIFIER_SINT) {
1382+
switch (length_mod) {
1383+
default:
1384+
case LENGTH_NONE:
1385+
case LENGTH_HH:
1386+
case LENGTH_H:
1387+
value->sint = va_arg(state->ap, int);
1388+
break;
1389+
case LENGTH_L:
1390+
value->sint = va_arg(state->ap, long);
1391+
break;
1392+
case LENGTH_LL:
1393+
value->sint =
1394+
(sint_value_type)va_arg(state->ap, long long);
1395+
break;
1396+
case LENGTH_J:
1397+
value->sint =
1398+
(sint_value_type)va_arg(state->ap, intmax_t);
1399+
break;
1400+
case LENGTH_Z: /* size_t */
1401+
case LENGTH_T: /* ptrdiff_t */
1402+
/* Though ssize_t is the signed equivalent of
1403+
* size_t for POSIX, there is no uptrdiff_t.
1404+
* Assume that size_t and ptrdiff_t are the
1405+
* unsigned and signed equivalents of each
1406+
* other. This can be checked in a platform
1407+
* test.
1408+
*/
1409+
value->sint =
1410+
(sint_value_type)va_arg(state->ap, ptrdiff_t);
1411+
break;
1412+
}
1413+
if (length_mod == LENGTH_HH) {
1414+
value->sint = (char)value->sint;
1415+
} else if (length_mod == LENGTH_H) {
1416+
value->sint = (short)value->sint;
1417+
}
1418+
} else if (specifier_cat == SPECIFIER_UINT) {
1419+
switch (length_mod) {
1420+
default:
1421+
case LENGTH_NONE:
1422+
case LENGTH_HH:
1423+
case LENGTH_H:
1424+
value->uint = va_arg(state->ap, unsigned int);
1425+
break;
1426+
case LENGTH_L:
1427+
value->uint = va_arg(state->ap, unsigned long);
1428+
break;
1429+
case LENGTH_LL:
1430+
value->uint =
1431+
(uint_value_type)va_arg(state->ap,
1432+
unsigned long long);
1433+
break;
1434+
case LENGTH_J:
1435+
value->uint =
1436+
(uint_value_type)va_arg(state->ap,
1437+
uintmax_t);
1438+
break;
1439+
case LENGTH_Z: /* size_t */
1440+
case LENGTH_T: /* ptrdiff_t */
1441+
value->uint =
1442+
(uint_value_type)va_arg(state->ap, size_t);
1443+
break;
1444+
}
1445+
if (length_mod == LENGTH_HH) {
1446+
value->uint = (unsigned char)value->uint;
1447+
} else if (length_mod == LENGTH_H) {
1448+
value->uint = (unsigned short)value->uint;
1449+
}
1450+
} else if (specifier_cat == SPECIFIER_FP) {
1451+
if (length_mod == LENGTH_UPPER_L) {
1452+
value->ldbl = va_arg(state->ap, long double);
1453+
} else {
1454+
value->dbl = va_arg(state->ap, double);
1455+
}
1456+
} else if (specifier_cat == SPECIFIER_PTR) {
1457+
value->ptr = va_arg(state->ap, void *);
1458+
}
1459+
}
1460+
12811461
int cbvprintf(cbprintf_cb out, void *ctx, const char *fp, va_list ap)
12821462
{
12831463
char buf[CONVERTED_BUFLEN];
12841464
size_t count = 0;
1465+
struct cbprintf_state state;
1466+
struct conversion *const conv = &state.conv;
1467+
union argument_value *const value = &state.value;
1468+
1469+
/* Make a copy of the arguments, because we can't pass ap to
1470+
* subroutines by value (caller wouldn't see the changes) nor
1471+
* by address (va_list may be an array type).
1472+
*/
1473+
va_copy(state.ap, ap);
12851474

12861475
/* Output character, returning EOF if output failed, otherwise
12871476
* updating count.
@@ -1316,169 +1505,10 @@ int cbvprintf(cbprintf_cb out, void *ctx, const char *fp, va_list ap)
13161505
continue;
13171506
}
13181507

1319-
/* Force union into RAM with conversion state to
1320-
* mitigate LLVM code generation bug.
1321-
*/
1322-
struct {
1323-
union argument_value value;
1324-
struct conversion conv;
1325-
} state = {
1326-
.value = {
1327-
.uint = 0,
1328-
},
1329-
};
1330-
struct conversion *const conv = &state.conv;
1331-
union argument_value *const value = &state.value;
13321508
const char *sp = fp;
1333-
int width = -1;
1334-
int precision = -1;
1335-
const char *bps = NULL;
1336-
const char *bpe = buf + sizeof(buf);
1337-
char sign = 0;
13381509

13391510
fp = extract_conversion(conv, sp);
1340-
1341-
/* If dynamic width is specified, process it,
1342-
* otherwise set with if present.
1343-
*/
1344-
if (conv->width_star) {
1345-
width = va_arg(ap, int);
1346-
1347-
if (width < 0) {
1348-
conv->flag_dash = true;
1349-
width = -width;
1350-
}
1351-
} else if (conv->width_present) {
1352-
width = conv->width_value;
1353-
}
1354-
1355-
/* If dynamic precision is specified, process it, otherwise
1356-
* set precision if present. For floating point where
1357-
* precision is not present use 6.
1358-
*/
1359-
if (conv->prec_star) {
1360-
int arg = va_arg(ap, int);
1361-
1362-
if (arg < 0) {
1363-
conv->prec_present = false;
1364-
} else {
1365-
precision = arg;
1366-
}
1367-
} else if (conv->prec_present) {
1368-
precision = conv->prec_value;
1369-
}
1370-
1371-
/* Reuse width and precision memory in conv for value
1372-
* padding counts.
1373-
*/
1374-
conv->pad0_value = 0;
1375-
conv->pad0_pre_exp = 0;
1376-
1377-
/* FP conversion requires knowing the precision. */
1378-
if (IS_ENABLED(CONFIG_CBPRINTF_FP_SUPPORT)
1379-
&& (conv->specifier_cat == SPECIFIER_FP)
1380-
&& !conv->prec_present) {
1381-
if (conv->specifier_a) {
1382-
precision = FRACTION_HEX;
1383-
} else {
1384-
precision = 6;
1385-
}
1386-
}
1387-
1388-
/* Get the value to be converted from the args.
1389-
*
1390-
* This can't be extracted to a helper function because
1391-
* passing a pointer to va_list doesn't work on x86_64. See
1392-
* https://stackoverflow.com/a/8048892.
1393-
*/
1394-
enum specifier_cat_enum specifier_cat
1395-
= (enum specifier_cat_enum)conv->specifier_cat;
1396-
enum length_mod_enum length_mod
1397-
= (enum length_mod_enum)conv->length_mod;
1398-
1399-
/* Extract the value based on the argument category and length.
1400-
*
1401-
* Note that the length modifier doesn't affect the value of a
1402-
* pointer argument.
1403-
*/
1404-
if (specifier_cat == SPECIFIER_SINT) {
1405-
switch (length_mod) {
1406-
default:
1407-
case LENGTH_NONE:
1408-
case LENGTH_HH:
1409-
case LENGTH_H:
1410-
value->sint = va_arg(ap, int);
1411-
break;
1412-
case LENGTH_L:
1413-
value->sint = va_arg(ap, long);
1414-
break;
1415-
case LENGTH_LL:
1416-
value->sint =
1417-
(sint_value_type)va_arg(ap, long long);
1418-
break;
1419-
case LENGTH_J:
1420-
value->sint =
1421-
(sint_value_type)va_arg(ap, intmax_t);
1422-
break;
1423-
case LENGTH_Z: /* size_t */
1424-
case LENGTH_T: /* ptrdiff_t */
1425-
/* Though ssize_t is the signed equivalent of
1426-
* size_t for POSIX, there is no uptrdiff_t.
1427-
* Assume that size_t and ptrdiff_t are the
1428-
* unsigned and signed equivalents of each
1429-
* other. This can be checked in a platform
1430-
* test.
1431-
*/
1432-
value->sint =
1433-
(sint_value_type)va_arg(ap, ptrdiff_t);
1434-
break;
1435-
}
1436-
if (length_mod == LENGTH_HH) {
1437-
value->sint = (char)value->sint;
1438-
} else if (length_mod == LENGTH_H) {
1439-
value->sint = (short)value->sint;
1440-
}
1441-
} else if (specifier_cat == SPECIFIER_UINT) {
1442-
switch (length_mod) {
1443-
default:
1444-
case LENGTH_NONE:
1445-
case LENGTH_HH:
1446-
case LENGTH_H:
1447-
value->uint = va_arg(ap, unsigned int);
1448-
break;
1449-
case LENGTH_L:
1450-
value->uint = va_arg(ap, unsigned long);
1451-
break;
1452-
case LENGTH_LL:
1453-
value->uint =
1454-
(uint_value_type)va_arg(ap,
1455-
unsigned long long);
1456-
break;
1457-
case LENGTH_J:
1458-
value->uint =
1459-
(uint_value_type)va_arg(ap,
1460-
uintmax_t);
1461-
break;
1462-
case LENGTH_Z: /* size_t */
1463-
case LENGTH_T: /* ptrdiff_t */
1464-
value->uint =
1465-
(uint_value_type)va_arg(ap, size_t);
1466-
break;
1467-
}
1468-
if (length_mod == LENGTH_HH) {
1469-
value->uint = (unsigned char)value->uint;
1470-
} else if (length_mod == LENGTH_H) {
1471-
value->uint = (unsigned short)value->uint;
1472-
}
1473-
} else if (specifier_cat == SPECIFIER_FP) {
1474-
if (length_mod == LENGTH_UPPER_L) {
1475-
value->ldbl = va_arg(ap, long double);
1476-
} else {
1477-
value->dbl = va_arg(ap, double);
1478-
}
1479-
} else if (specifier_cat == SPECIFIER_PTR) {
1480-
value->ptr = va_arg(ap, void *);
1481-
}
1511+
pull_va_args(&state);
14821512

14831513
/* We've now consumed all arguments related to this
14841514
* specification. If the conversion is invalid, or is
@@ -1490,6 +1520,12 @@ int cbvprintf(cbprintf_cb out, void *ctx, const char *fp, va_list ap)
14901520
continue;
14911521
}
14921522

1523+
int width = state.width;
1524+
int precision = state.precision;
1525+
const char *bps = NULL;
1526+
const char *bpe = buf + sizeof(buf);
1527+
char sign = 0;
1528+
14931529
/* Do formatting, either into the buffer or
14941530
* referencing external data.
14951531
*/

0 commit comments

Comments
 (0)