Message ID | 20240430192739.1032549-3-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | More tunable fixes | expand |
On 2024-04-30 15:25, Adhemerval Zanella wrote: > And move it to parse_tunables. It avoids a string comparison for > each tunable. > > Checked on aarch64-linux-gnu and x86_64-linux-gnu. > --- > elf/dl-tunables.c | 58 +++++++++++++++++++++++++++++++---------------- > 1 file changed, 38 insertions(+), 20 deletions(-) > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c > index 1db80e0f92..63cf8c7ab5 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -119,6 +119,17 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp, > cur->initialized = true; > } > > +static bool > +tunable_parse_num (const char *strval, size_t len, tunable_num_t *val) > +{ > + char *endptr = NULL; > + uint64_t numval = _dl_strtoul (strval, &endptr); > + if (endptr != strval + len) > + return false; > + *val = (tunable_num_t) numval; > + return true; > +} > + > /* Validate range of the input value and initialize the tunable CUR if it looks > good. */ > static bool > @@ -128,11 +139,8 @@ tunable_initialize (tunable_t *cur, const char *strval, size_t len) > > if (cur->type.type_code != TUNABLE_TYPE_STRING) > { > - char *endptr = NULL; > - uint64_t numval = _dl_strtoul (strval, &endptr); > - if (endptr != strval + len) > + if (!tunable_parse_num (strval, len, &val.numval)) Refactoring. OK. > return false; > - val.numval = (tunable_num_t) numval; > } > else > val.strval = (struct tunable_str_t) { strval, len }; > @@ -223,17 +231,6 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) > if (tunable_is_name (cur->name, name)) > { > tunables[i] = (struct tunable_toset_t) { cur, value, p - value }; > - > - /* Ignore tunables if enable_secure is set */ > - if (tunable_is_name ("glibc.rtld.enable_secure", name)) > - { > - tunable_num_t val = (tunable_num_t) _dl_strtoul (value, NULL); > - if (val == 1) > - { > - __libc_enable_secure = 1; > - return 0; > - } > - } Drop this string comparison for a tunable comparison below. OK. > break; > } > } > @@ -242,6 +239,16 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) > return ntunables; > } > > +static void > +parse_tunable_print_error (const struct tunable_toset_t *toset) > +{ > + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " > + "for option `%s': ignored.\n", > + (int) toset->len, > + toset->value, > + toset->t->name); > +} > + Refactor. OK. > static void > parse_tunables (const char *valstring) > { > @@ -253,6 +260,21 @@ parse_tunables (const char *valstring) > return; > } > > + /* Ignore tunables if enable_secure is set */ > + struct tunable_toset_t *tsec = > + &tunables[TUNABLE_ENUM_NAME(glibc, rtld, enable_secure)]; > + if (tsec->t != NULL) > + { > + tunable_num_t val; > + if (!tunable_parse_num (tsec->value, tsec->len, &val)) > + parse_tunable_print_error (tsec); > + else if (val == 1) > + { > + __libc_enable_secure = 1; > + return; > + } > + } > + Directly read the enable_secure tunable when it is set. OK. > for (int i = 0; i < tunables_list_size; i++) > { > if (tunables[i].t == NULL) > @@ -260,11 +282,7 @@ parse_tunables (const char *valstring) > > if (!tunable_initialize (tunables[i].t, tunables[i].value, > tunables[i].len)) > - _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " > - "for option `%s': ignored.\n", > - (int) tunables[i].len, > - tunables[i].value, > - tunables[i].t->name); > + parse_tunable_print_error (&tunables[i]); > } > } > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index 1db80e0f92..63cf8c7ab5 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -119,6 +119,17 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp, cur->initialized = true; } +static bool +tunable_parse_num (const char *strval, size_t len, tunable_num_t *val) +{ + char *endptr = NULL; + uint64_t numval = _dl_strtoul (strval, &endptr); + if (endptr != strval + len) + return false; + *val = (tunable_num_t) numval; + return true; +} + /* Validate range of the input value and initialize the tunable CUR if it looks good. */ static bool @@ -128,11 +139,8 @@ tunable_initialize (tunable_t *cur, const char *strval, size_t len) if (cur->type.type_code != TUNABLE_TYPE_STRING) { - char *endptr = NULL; - uint64_t numval = _dl_strtoul (strval, &endptr); - if (endptr != strval + len) + if (!tunable_parse_num (strval, len, &val.numval)) return false; - val.numval = (tunable_num_t) numval; } else val.strval = (struct tunable_str_t) { strval, len }; @@ -223,17 +231,6 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) if (tunable_is_name (cur->name, name)) { tunables[i] = (struct tunable_toset_t) { cur, value, p - value }; - - /* Ignore tunables if enable_secure is set */ - if (tunable_is_name ("glibc.rtld.enable_secure", name)) - { - tunable_num_t val = (tunable_num_t) _dl_strtoul (value, NULL); - if (val == 1) - { - __libc_enable_secure = 1; - return 0; - } - } break; } } @@ -242,6 +239,16 @@ parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables) return ntunables; } +static void +parse_tunable_print_error (const struct tunable_toset_t *toset) +{ + _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " + "for option `%s': ignored.\n", + (int) toset->len, + toset->value, + toset->t->name); +} + static void parse_tunables (const char *valstring) { @@ -253,6 +260,21 @@ parse_tunables (const char *valstring) return; } + /* Ignore tunables if enable_secure is set */ + struct tunable_toset_t *tsec = + &tunables[TUNABLE_ENUM_NAME(glibc, rtld, enable_secure)]; + if (tsec->t != NULL) + { + tunable_num_t val; + if (!tunable_parse_num (tsec->value, tsec->len, &val)) + parse_tunable_print_error (tsec); + else if (val == 1) + { + __libc_enable_secure = 1; + return; + } + } + for (int i = 0; i < tunables_list_size; i++) { if (tunables[i].t == NULL) @@ -260,11 +282,7 @@ parse_tunables (const char *valstring) if (!tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len)) - _dl_error_printf ("WARNING: ld.so: invalid GLIBC_TUNABLES value `%.*s' " - "for option `%s': ignored.\n", - (int) tunables[i].len, - tunables[i].value, - tunables[i].t->name); + parse_tunable_print_error (&tunables[i]); } }