* [PATCH] stdio-common/printf-prs.c: Simplify test
@ 2021-03-24 18:07 Alejandro Colomar
2021-03-29 19:32 ` Adhemerval Zanella
0 siblings, 1 reply; 4+ messages in thread
From: Alejandro Colomar @ 2021-03-24 18:07 UTC (permalink / raw)
To: libc-alpha; +Cc: Alejandro Colomar
The test was being repeated for all the remaining code in the for loop.
Test once, and 'continue' to next iteration if necessary, so that the
following code can assume that the necessary condition is met.
Minor changes: fix some mixed tabs & spaces to use spaces only.
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---
stdio-common/printf-prs.c | 44 ++++++++++++++++++++-------------------
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/stdio-common/printf-prs.c b/stdio-common/printf-prs.c
index 1d4e00553d..a0c5c5487b 100644
--- a/stdio-common/printf-prs.c
+++ b/stdio-common/printf-prs.c
@@ -72,30 +72,32 @@ parse_printf_format (const char *fmt, size_t n, int *argtypes)
/* Parse this spec. */
nargs += __parse_one_specmb (f, nargs, &spec, &max_ref_arg);
+ if ((size_t) spec.width_arg >= n)
+ continue;
+
/* If the width is determined by an argument this is an int. */
- if (spec.width_arg != -1 && (size_t) spec.width_arg < n)
- argtypes[spec.width_arg] = PA_INT;
+ if (spec.width_arg != -1)
+ argtypes[spec.width_arg] = PA_INT;
/* If the precision is determined by an argument this is an int. */
- if (spec.prec_arg != -1 && (size_t) spec.prec_arg < n)
- argtypes[spec.prec_arg] = PA_INT;
-
- if ((size_t) spec.data_arg < n)
- switch (spec.ndata_args)
- {
- case 0: /* No arguments. */
- break;
- case 1: /* One argument; we already have the type. */
- argtypes[spec.data_arg] = spec.data_arg_type;
- break;
- default:
- /* We have more than one argument for this format spec. We must
- call the arginfo function again to determine all the types. */
- (void) (*__printf_arginfo_table[spec.info.spec])
- (&spec.info, n - spec.data_arg, &argtypes[spec.data_arg],
- &spec.size);
- break;
- }
+ if (spec.prec_arg != -1)
+ argtypes[spec.prec_arg] = PA_INT;
+
+ switch (spec.ndata_args)
+ {
+ case 0: /* No arguments. */
+ break;
+ case 1: /* One argument; we already have the type. */
+ argtypes[spec.data_arg] = spec.data_arg_type;
+ break;
+ default:
+ /* We have more than one argument for this format spec. We must
+ call the arginfo function again to determine all the types. */
+ (void) (*__printf_arginfo_table[spec.info.spec])
+ (&spec.info, n - spec.data_arg, &argtypes[spec.data_arg],
+ &spec.size);
+ break;
+ }
}
return MAX (nargs, max_ref_arg);
--
2.31.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] stdio-common/printf-prs.c: Simplify test
2021-03-24 18:07 [PATCH] stdio-common/printf-prs.c: Simplify test Alejandro Colomar
@ 2021-03-29 19:32 ` Adhemerval Zanella
2021-03-29 20:35 ` Alejandro Colomar (man-pages)
0 siblings, 1 reply; 4+ messages in thread
From: Adhemerval Zanella @ 2021-03-29 19:32 UTC (permalink / raw)
To: Alejandro Colomar, libc-alpha
On 24/03/2021 15:07, Alejandro Colomar via Libc-alpha wrote:
> The test was being repeated for all the remaining code in the for loop.
> Test once, and 'continue' to next iteration if necessary, so that the
> following code can assume that the necessary condition is met.
>
> Minor changes: fix some mixed tabs & spaces to use spaces only.
>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
We do not use SCO, but rather Copyright assignment. And I think we will
need to add a testcase for this change, this simple program below shows
a complete different result with this patch applied:
---
x86_64-linux-gnu$ cat test.c
#include <stdio.h>
#include <printf.h>
#include <sys/param.h>
static void
print_argtype (int argtype)
{
if ((argtype & ~PA_FLAG_MASK) == PA_INT)
printf ("PA_INT");
else if ((argtype & ~PA_FLAG_MASK) == PA_CHAR)
printf ("PA_CHAR");
else if ((argtype & ~PA_FLAG_MASK) == PA_STRING)
printf ("PA_STRING");
else if ((argtype & ~PA_FLAG_MASK) == PA_POINTER)
printf ("PA_POINTER");
else if ((argtype & ~PA_FLAG_MASK) == PA_FLOAT)
printf ("PA_FLOAT");
else if ((argtype & ~PA_FLAG_MASK) == PA_DOUBLE)
printf ("PA_DOUBLE");
else
printf ("UNKNOWN");
if ((argtype & PA_FLAG_MASK) == PA_FLAG_PTR)
printf ("| PA_FLAG_PTR");
if ((argtype & PA_FLAG_MASK) == PA_FLAG_SHORT)
printf ("| PA_FLAG_SHORT");
if ((argtype & PA_FLAG_MASK) == PA_FLAG_LONG)
printf (" | PA_FLAG_LONG");
if ((argtype & PA_FLAG_MASK) == PA_FLAG_LONG_LONG)
printf (" | PA_FLAG_LONG_LONG");
if ((argtype & PA_FLAG_MASK) == PA_FLAG_LONG_DOUBLE)
printf (" | PA_FLAG_LONG_DOUBLE");
printf ("\n");
}
static void
test_parse_printf_format (int maxsize)
{
int argtypes[maxsize];
size_t argtypes_s = sizeof (argtypes) / sizeof (argtypes[0]);
size_t s = parse_printf_format (
"%10u foo %% %+2.3f %ld %s %lld %jd %zu %ld",
argtypes_s, argtypes);
for (int i = 0; i < MIN (s, argtypes_s); i++)
print_argtype (argtypes[i]);
printf ("\n");
}
int main (int argc, char *argv[])
{
test_parse_printf_format (4);
test_parse_printf_format (20);
return 0;
}
x86_64-linux-gnu$ gcc -Wall test.c -o test
x86_64-linux-gnu$ ./test
PA_INT
PA_DOUBLE
PA_INT | PA_FLAG_LONG
PA_STRING
PA_INT
PA_DOUBLE
PA_INT | PA_FLAG_LONG
PA_STRING
PA_INT | PA_FLAG_LONG
PA_INT | PA_FLAG_LONG
PA_INT | PA_FLAG_LONG
PA_INT | PA_FLAG_LONG
x86_64-linux-gnu$ ./testrun.sh ./test
PA_INT
PA_INT
PA_INT
PA_INT
UNKNOWN
UNKNOWN
UNKNOWN
UNKNOWN
UNKNOWN
UNKNOWN
UNKNOWN
UNKNOWN
> ---
> stdio-common/printf-prs.c | 44 ++++++++++++++++++++-------------------
> 1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/stdio-common/printf-prs.c b/stdio-common/printf-prs.c
> index 1d4e00553d..a0c5c5487b 100644
> --- a/stdio-common/printf-prs.c
> +++ b/stdio-common/printf-prs.c
> @@ -72,30 +72,32 @@ parse_printf_format (const char *fmt, size_t n, int *argtypes)
> /* Parse this spec. */
> nargs += __parse_one_specmb (f, nargs, &spec, &max_ref_arg);
>
> + if ((size_t) spec.width_arg >= n)
> + continue;
> +
> /* If the width is determined by an argument this is an int. */
> - if (spec.width_arg != -1 && (size_t) spec.width_arg < n)
> - argtypes[spec.width_arg] = PA_INT;
> + if (spec.width_arg != -1)
> + argtypes[spec.width_arg] = PA_INT;
>
> /* If the precision is determined by an argument this is an int. */
> - if (spec.prec_arg != -1 && (size_t) spec.prec_arg < n)
> - argtypes[spec.prec_arg] = PA_INT;
> -
> - if ((size_t) spec.data_arg < n)
> - switch (spec.ndata_args)
> - {
> - case 0: /* No arguments. */
> - break;
> - case 1: /* One argument; we already have the type. */
> - argtypes[spec.data_arg] = spec.data_arg_type;
> - break;
> - default:
> - /* We have more than one argument for this format spec. We must
> - call the arginfo function again to determine all the types. */
> - (void) (*__printf_arginfo_table[spec.info.spec])
> - (&spec.info, n - spec.data_arg, &argtypes[spec.data_arg],
> - &spec.size);
> - break;
> - }
> + if (spec.prec_arg != -1)
> + argtypes[spec.prec_arg] = PA_INT;
> +
> + switch (spec.ndata_args)
> + {
> + case 0: /* No arguments. */
> + break;
> + case 1: /* One argument; we already have the type. */
> + argtypes[spec.data_arg] = spec.data_arg_type;
> + break;
> + default:
> + /* We have more than one argument for this format spec. We must
> + call the arginfo function again to determine all the types. */
> + (void) (*__printf_arginfo_table[spec.info.spec])
> + (&spec.info, n - spec.data_arg, &argtypes[spec.data_arg],
> + &spec.size);
> + break;
> + }
> }
>
> return MAX (nargs, max_ref_arg);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] stdio-common/printf-prs.c: Simplify test
2021-03-29 19:32 ` Adhemerval Zanella
@ 2021-03-29 20:35 ` Alejandro Colomar (man-pages)
2021-03-29 20:43 ` Adhemerval Zanella
0 siblings, 1 reply; 4+ messages in thread
From: Alejandro Colomar (man-pages) @ 2021-03-29 20:35 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 3/29/21 9:32 PM, Adhemerval Zanella wrote:
>
>
> On 24/03/2021 15:07, Alejandro Colomar via Libc-alpha wrote:
>> The test was being repeated for all the remaining code in the for loop.
>> Test once, and 'continue' to next iteration if necessary, so that the
>> following code can assume that the necessary condition is met.
>>
>> Minor changes: fix some mixed tabs & spaces to use spaces only.
>>
>> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
>
> We do not use SCO, but rather Copyright assignment. And I think we will
> need to add a testcase for this change, this simple program below shows
> a complete different result with this patch applied:
>
Hello Adhemerval,
Yes, I was completely wrong with that patch. I submitted a patch v2
after that, trying to improve the comments (without the code changes,
which were wrong) to avoid confusion:
<https://sourceware.org/pipermail/libc-alpha/2021-March/124359.html>.
Thanks,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] stdio-common/printf-prs.c: Simplify test
2021-03-29 20:35 ` Alejandro Colomar (man-pages)
@ 2021-03-29 20:43 ` Adhemerval Zanella
0 siblings, 0 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2021-03-29 20:43 UTC (permalink / raw)
To: Alejandro Colomar (man-pages), libc-alpha
On 29/03/2021 17:35, Alejandro Colomar (man-pages) wrote:
>
>
> On 3/29/21 9:32 PM, Adhemerval Zanella wrote:
>>
>>
>> On 24/03/2021 15:07, Alejandro Colomar via Libc-alpha wrote:
>>> The test was being repeated for all the remaining code in the for loop.
>>> Test once, and 'continue' to next iteration if necessary, so that the
>>> following code can assume that the necessary condition is met.
>>>
>>> Minor changes: fix some mixed tabs & spaces to use spaces only.
>>>
>>> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
>>
>> We do not use SCO, but rather Copyright assignment. And I think we will
>> need to add a testcase for this change, this simple program below shows
>> a complete different result with this patch applied:
>>
>
> Hello Adhemerval,
>
> Yes, I was completely wrong with that patch. I submitted a patch v2
> after that, trying to improve the comments (without the code changes,
> which were wrong) to avoid confusion:
> <https://sourceware.org/pipermail/libc-alpha/2021-March/124359.html>.
Ok, I will review this one and mark this as superseded.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-29 20:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 18:07 [PATCH] stdio-common/printf-prs.c: Simplify test Alejandro Colomar
2021-03-29 19:32 ` Adhemerval Zanella
2021-03-29 20:35 ` Alejandro Colomar (man-pages)
2021-03-29 20:43 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).