> Interesting -- so you're lazily constructing the nl_arg data when you > first discover the need for it, rather than unconditionally doing the > first pass to look for it. Do you measure a relevant performance > difference doing this? Yes, the benchmark show there are about  30% benefits for commonly used format string. BM_stdio_printf_s, 160 ns（before opt）, 110 ns （after opt） static void BM_stdio_printf_s(benchmark::State& state) {                                                            while (state.KeepRunning()) {                                                                                           char buf[BUFSIZ];                                                                                                    snprintf(buf, sizeof(buf), "this is a more typical error message with detail: %s",                                            "No such file or directory");     }                                                                                                               }                                                                                                                 MUSL_BENCHMARK(BM_stdio_printf_s); > Stylistically, I like that the patch is minimally invasive and does > not make complex changes (like what I suggested above) together with > the intended functional change. I don't really like building up extra > "special meaning" state like whether nl_arg_ptr is null on top of > whether f is null to represent what mode the printf_core is operating > in, but this can/should probably be cleaned up as a separate patch to > keep the functional change itself simple, like what you've done. I add nl_arg_filled to indicate whether we have completed the acquisition of parameters. > On a technical note, some compilers are bad about hoisting large local > objects out of their scopes when inlining. This results in the large > floating point buffer from fmt_fp getting allocated at the top of > printf_core, and presumably causes a recursive call to printf_core to > double the stack requirements here, which is problematic. I'm not sure > if we should try to avoid recursion (it should be fairly > straightforward to do so) or just try to fix the unwanted hoisting to > begin with (something I've been wanting to do for a while). I modified buf to be declared when using ranther than at the top of printf_core. > It looks like these two hunks are almost not needed, since it's not > allowed to mix positional and non-positional forms.  Thanks， I had changed it in patch. So musl don't support these format strings, right? printf("yc test: %*2$d \n", 2, 10); printf("yc test: %.*2$f \n", 1.123456, 10); Does "%n$d", "%*n$d" or "%.*n$d" should be used together? > However, %m does > not take an argument, but might have width/precision in * form. It'd > be nice if there were some way to avoid this duplication of logic. Do you mean "%*m$" or "%.*m$"? What do you mean "duplication of logic"? I don't get it. > No check has been made that you haven't already consumed arguments via > non-positional-form format specifiers. This will make things blow up > much worse if the caller wrongly mixes them, consuming wrong-type args > from the wrong positions rather than erroring out (as we do now) or > trapping (probably ideal). Right now we don't actually catch all forms > of mixing either, but we do necessarily consume all positional args in > order before attempting to consume any erroneously-specified > non-positional ones. Your patch changes this to happen in the order > things were encountered. So I think, as a first step before even doing > this patch, we should update the l10n flag to a tri-state thing where > it can be yes/no/indeterminate, to allow erroring out on any specifier > that mismatches the existing yes/no. I remember you mentioned "calling printf with an invalid format string has undefined behavior" here, so maybe it's also ok. https://www.openwall.com/lists/musl/2023/05/07/1 Chuang Yin ------------------ 原始邮件 ------------------ 发件人: "Rich Felker"