>From 896554fd01d217ad4d823d883606377cb6d76124 Mon Sep 17 00:00:00 2001 From: Bernhard Voelker Date: Tue, 3 Dec 2013 00:38:15 +0100 Subject: [PATCH] maint: avoid '%s' quoting notation in diagnostic messages Add a new rule to ensure the use of quote() instead of '%s' or `%s' in format strings of diagnostics messages. * cfg.mk (sc_prohibit_quotes_notation): Add rule. * TODO: Remove the entry regarding the '%s' notation. * src/mkfifo.c (main): Remove the offending and in this case even duplicate quoting in the format string of the error diagnostic. * src/mknod.c (main): Likewise. * src/df.c (decode_output_arg): Change two invocations of error() according to the above new rule. * src/numfmt.c: Fix numerous wrong quote notations to fit the above new rule, mostly in internal debugging diagnostic messages. (extract_fields): In this function, avoid calling quote() for *prefix, *_data and *_suffix as these may be NULL; instead, change the format string so that it will not be detected by the above new rule. --- TODO | 4 ---- cfg.mk | 8 ++++++++ src/df.c | 6 +++--- src/mkfifo.c | 2 +- src/mknod.c | 2 +- src/numfmt.c | 58 +++++++++++++++++++++++++++++++++------------------------- 6 files changed, 46 insertions(+), 34 deletions(-) diff --git a/TODO b/TODO index e10da7c..3dc05af 100644 --- a/TODO +++ b/TODO @@ -130,10 +130,6 @@ Add a distcheck-time test to ensure that every distributed file is either read-only(indicating generated) or is version-controlled and up to date. -remove '%s' notation (now that they're all gone, add a maint.mk sc_ - rule to ensure no new ones are added): - grep -E "\`%.{,4}s'" src/*.c - remove all uses of the 'register' keyword: Done. add a maint.mk rule for this, too. diff --git a/cfg.mk b/cfg.mk index 788c351..9e8e8ac 100644 --- a/cfg.mk +++ b/cfg.mk @@ -151,6 +151,14 @@ sc_system_h_headers: .re-list 1>&2; exit 1; } || :; \ fi +# Files in src/ should not use '%s' notation in format strings, +# i.e., single quotes around %s (or similar) should be avoided. +sc_prohibit_quotes_notation: + @cd $(srcdir)/src && GIT_PAGER= git grep -n "\".*[\`']%s'.*\"" *.c \ + && { echo '$(ME): '"Use quote() to avoid quoted '%s' notation" 1>&2; \ + exit 1; } \ + || : + sc_sun_os_names: @grep -nEi \ 'solaris[^[:alnum:]]*2\.(7|8|9|[1-9][0-9])|sunos[^[:alnum:]][6-9]' \ diff --git a/src/df.c b/src/df.c index f6ce79d..ba8ef15 100644 --- a/src/df.c +++ b/src/df.c @@ -383,15 +383,15 @@ decode_output_arg (char const *arg) } if (field == -1) { - error (0, 0, _("option --output: field '%s' unknown"), s); + error (0, 0, _("option --output: field %s unknown"), quote (s)); usage (EXIT_FAILURE); } if (field_data[field].used) { /* Prevent the fields from being used more than once. */ - error (0, 0, _("option --output: field '%s' used more than once"), - field_data[field].arg); + error (0, 0, _("option --output: field %s used more than once"), + quote (field_data[field].arg)); usage (EXIT_FAILURE); } diff --git a/src/mkfifo.c b/src/mkfifo.c index df97d6b..2428dd0 100644 --- a/src/mkfifo.c +++ b/src/mkfifo.c @@ -170,7 +170,7 @@ main (int argc, char **argv) } else if (specified_mode && lchmod (argv[optind], newmode) != 0) { - error (0, errno, _("cannot set permissions of `%s'"), + error (0, errno, _("cannot set permissions of %s"), quote (argv[optind])); exit_status = EXIT_FAILURE; } diff --git a/src/mknod.c b/src/mknod.c index 908d840..7856e51 100644 --- a/src/mknod.c +++ b/src/mknod.c @@ -265,7 +265,7 @@ main (int argc, char **argv) } if (specified_mode && lchmod (argv[optind], newmode) != 0) - error (EXIT_FAILURE, errno, _("cannot set permissions of `%s'"), + error (EXIT_FAILURE, errno, _("cannot set permissions of %s"), quote (argv[optind])); exit (EXIT_SUCCESS); diff --git a/src/numfmt.c b/src/numfmt.c index a809949..b29d16f 100644 --- a/src/numfmt.c +++ b/src/numfmt.c @@ -598,8 +598,9 @@ simple_strtod_human (const char *input_str, /* 'scale_auto' is checked below. */ int scale_base = default_scale_base (allowed_scaling); - devmsg ("simple_strtod_human:\n input string: '%s'\n " - "locale decimal-point: '%s'\n", input_str, decimal_point); + devmsg ("simple_strtod_human:\n input string: %s\n " + "locale decimal-point: %s\n", + quote_n (0, input_str), quote_n (1, decimal_point)); enum simple_strtod_error e = simple_strtod_float (input_str, endptr, value, precision); @@ -673,29 +674,29 @@ simple_strtod_fatal (enum simple_strtod_error err, char const *input_str) abort (); case SSE_OVERFLOW: - msgid = N_("value too large to be converted: '%s'"); + msgid = N_("value too large to be converted: %s"); break; case SSE_INVALID_NUMBER: - msgid = N_("invalid number: '%s'"); + msgid = N_("invalid number: %s"); break; case SSE_VALID_BUT_FORBIDDEN_SUFFIX: - msgid = N_("rejecting suffix in input: '%s' (consider using --from)"); + msgid = N_("rejecting suffix in input: %s (consider using --from)"); break; case SSE_INVALID_SUFFIX: - msgid = N_("invalid suffix in input: '%s'"); + msgid = N_("invalid suffix in input: %s"); break; case SSE_MISSING_I_SUFFIX: - msgid = N_("missing 'i' suffix in input: '%s' (e.g Ki/Mi/Gi)"); + msgid = N_("missing 'i' suffix in input: %s (e.g Ki/Mi/Gi)"); break; } if (_invalid != inval_ignore) - error (conv_exit_code, 0, gettext (msgid), input_str); + error (conv_exit_code, 0, gettext (msgid), quote (input_str)); } /* Convert VAL to a human format string in BUF. */ @@ -766,7 +767,7 @@ double_to_human (long double val, int precision, if (scale == scale_IEC_I && power > 0) strncat (buf, "i", buf_size - strlen (buf) - 1); - devmsg (" returning value: '%s'\n", buf); + devmsg (" returning value: %s\n", quote (buf)); return; } @@ -784,7 +785,7 @@ unit_to_umax (const char *n_string) s_err = xstrtoumax (n_string, &end, 10, &n, "KMGTPEZY"); if (s_err != LONGINT_OK || *end || n == 0) - error (EXIT_FAILURE, 0, _("invalid unit size: '%s'"), n_string); + error (EXIT_FAILURE, 0, _("invalid unit size: %s"), quote (n_string)); return n; } @@ -1035,11 +1036,11 @@ parse_format_string (char const *fmt) devmsg ("format String:\n input: %s\n grouping: %s\n" " padding width: %ld\n alignment: %s\n" - " prefix: '%s'\n suffix: '%s'\n", + " prefix: %s\n suffix: %s\n", quote (fmt), (grouping) ? "yes" : "no", padding_width, (padding_alignment == MBS_ALIGN_LEFT) ? "Left" : "Right", - format_str_prefix, format_str_suffix); + quote_n (0, format_str_prefix), quote_n (1, format_str_suffix)); } /* Parse a numeric value (with optional suffix) from a string. @@ -1067,8 +1068,8 @@ parse_human_number (const char *str, long double /*output */ *value, if (ptr && *ptr != '\0') { if (_invalid != inval_ignore) - error (conv_exit_code, 0, _("invalid suffix in input '%s': '%s'"), - str, ptr); + error (conv_exit_code, 0, _("invalid suffix in input %s: %s"), + quote (str), quote (ptr)); e = SSE_INVALID_SUFFIX; } return e; @@ -1107,7 +1108,8 @@ prepare_padded_number (const long double val, size_t precision) if (suffix) strncat (buf, suffix, sizeof (buf) - strlen (buf) -1); - devmsg ("formatting output:\n value: %Lf\n humanized: '%s'\n", val, buf); + devmsg ("formatting output:\n value: %Lf\n humanized: %s\n", + val, quote (buf)); if (padding_width && strlen (buf) < padding_width) { @@ -1115,7 +1117,7 @@ prepare_padded_number (const long double val, size_t precision) mbsalign (buf, padding_buffer, padding_buffer_size, &w, padding_alignment, MBA_UNIBYTE_ONLY); - devmsg (" After padding: '%s'\n", padding_buffer); + devmsg (" After padding: %s\n", quote (padding_buffer)); } else { @@ -1151,7 +1153,7 @@ process_suffixed_number (char *text, long double *result, size_t *precision) { /* trim suffix, ONLY if it's at the end of the text. */ *possible_suffix = '\0'; - devmsg ("trimming suffix '%s'\n", suffix); + devmsg ("trimming suffix %s\n", quote (suffix)); } else devmsg ("no valid suffix found\n"); @@ -1181,7 +1183,8 @@ process_suffixed_number (char *text, long double *result, size_t *precision) long double val = 0; enum simple_strtod_error e = parse_human_number (p, &val, precision); if (e == SSE_OK_PRECISION_LOSS && debug) - error (0, 0, _("large input value '%s': possible precision loss"), p); + error (0, 0, _("large input value %s: possible precision loss"), + quote (p)); if (from_unit_size != 1 || to_unit_size != 1) val = (val * from_unit_size) / to_unit_size; @@ -1241,7 +1244,8 @@ extract_fields (char *line, int _field, *_data = NULL; *_suffix = NULL; - devmsg ("extracting Fields:\n input: '%s'\n field: %d\n", line, _field); + devmsg ("extracting Fields:\n input: %s\n field: %d\n", + quote (line), _field); if (field > 1) { @@ -1251,7 +1255,7 @@ extract_fields (char *line, int _field, if (*ptr == '\0') { /* not enough fields in the input - print warning? */ - devmsg (" TOO FEW FIELDS!\n prefix: '%s'\n", *_prefix); + devmsg (" TOO FEW FIELDS!\n prefix: %s\n", quote (*_prefix)); return; } @@ -1271,7 +1275,9 @@ extract_fields (char *line, int _field, else *_suffix = NULL; - devmsg (" prefix: '%s'\n number: '%s'\n suffix: '%s'\n", + /* Although the quoting notation '%s' violates sc_prohibit_quotes_notation, + avoid using quote() here as e.g. *_prefix may be NULL. */ + devmsg (" prefix: '%s""'\n number: '%s""'\n suffix: '%s""'\n", *_prefix, *_data, *_suffix); } @@ -1384,7 +1390,8 @@ main (int argc, char **argv) case PADDING_OPTION: if (xstrtol (optarg, NULL, 10, &padding_width, "") != LONGINT_OK || padding_width == 0) - error (EXIT_FAILURE, 0, _("invalid padding value '%s'"), optarg); + error (EXIT_FAILURE, 0, _("invalid padding value %s"), + quote (optarg)); if (padding_width < 0) { padding_alignment = MBS_ALIGN_LEFT; @@ -1397,7 +1404,8 @@ main (int argc, char **argv) case FIELD_OPTION: if (xstrtol (optarg, NULL, 10, &field, "") != LONGINT_OK || field <= 0) - error (EXIT_FAILURE, 0, _("invalid field value '%s'"), optarg); + error (EXIT_FAILURE, 0, _("invalid field value %s"), + quote (optarg)); break; case 'd': @@ -1426,8 +1434,8 @@ main (int argc, char **argv) { if (xstrtoumax (optarg, NULL, 10, &header, "") != LONGINT_OK || header == 0) - error (EXIT_FAILURE, 0, _("invalid header value '%s'"), - optarg); + error (EXIT_FAILURE, 0, _("invalid header value %s"), + quote (optarg)); } else { -- 1.8.3.1