[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] maint: prohibit an operator at end of line
|
From: |
Jim Meyering |
|
Subject: |
Re: [PATCH 3/3] maint: prohibit an operator at end of line |
|
Date: |
Wed, 02 May 2012 10:35:36 +0200 |
Eric Blake wrote:
> On 04/30/2012 08:25 AM, Eric Blake wrote:
>> On 04/30/2012 07:08 AM, Jim Meyering wrote:
>>> Here's a new syntax-check rule (patch 3/3).
>>> 1 and 2 fix violations so that the new check passes.
>>>
>>> For now, this is only in coreutils (grep had only two violations),
>>> but given the long list of violations in gnulib's lib/ directory,
>>> it may not be generally appealing enough to be added to gnulib's maint.mk:
>>>
>>> $ git grep -lE '(. [-/+]|&&|\|\|)$' lib
>>
>> Is it worth adding some more operators, such as *, ^, &, and |?
>>
>> (. [-/+*^]|&&?|\|\|?)$
>
> Also <<, >>, ==, !=, and for that matter, all of the op= operators:
>
> (. [-/+*^=!<>]=?|&&?|\|\|?|<<=?|>>=?)$
Good point.
Here's a more complete patch that also makes the
regular expression overridable.
I'm still omitting "*" and "=" from the default.
>From b8a6996e258a2c30de40fb20cab0d17a38c3eff2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 1 Jan 2012 01:46:34 +0100
Subject: [PATCH 1/3] maint: with split lines, don't leave an operator at end
of line
* src/copy.c (copy_reg): Split an expression before a binary operator,
not after it.
* src/cut.c (set_fields): Likewise.
* src/id.c (main): Likewise.
* src/install.c (setdefaultfilecon): Likewise.
* src/join.c (ignore_case): Likewise.
* src/pr.c (cols_ready_to_print, init_parameters, print_page): Likewise.
* src/stty.c (set_window_size): Likewise.
* src/wc.c (SUPPORT_OLD_MBRTOWC): Likewise.
* src/who.c (scan_entries): Likewise.
* src/test.c (binary_operator): Join a split line.
* src/extent-scan.c (extent_scan_read): Move an ">" from end of line
to beginning of the following.
Likewise for two other expressions.
---
src/copy.c | 4 ++--
src/cut.c | 4 ++--
src/extent-scan.c | 12 ++++++------
src/id.c | 4 ++--
src/install.c | 4 ++--
src/join.c | 2 +-
src/pr.c | 22 +++++++++++-----------
src/stty.c | 4 ++--
src/test.c | 4 ++--
src/wc.c | 2 +-
src/who.c | 6 +++---
11 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 414fbe0..26bbcf2 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1055,8 +1055,8 @@ copy_reg (char const *src_name, char const *dst_name,
make_holes, src_name, dst_name,
UINTMAX_MAX, &n_read,
&wrote_hole_at_eof)
- || (wrote_hole_at_eof &&
- ftruncate (dest_desc, n_read) < 0))
+ || (wrote_hole_at_eof
+ && ftruncate (dest_desc, n_read) < 0))
{
error (0, errno, _("failed to extend %s"), quote (dst_name));
return_val = false;
diff --git a/src/cut.c b/src/cut.c
index fed9616..87380ac 100644
--- a/src/cut.c
+++ b/src/cut.c
@@ -372,8 +372,8 @@ set_fields (const char *fieldstr)
initial = (lhs_specified ? value : 1);
value = 0;
}
- else if (*fieldstr == ',' ||
- isblank (to_uchar (*fieldstr)) || *fieldstr == '\0')
+ else if (*fieldstr == ','
+ || isblank (to_uchar (*fieldstr)) || *fieldstr == '\0')
{
in_digits = false;
/* Ending the string, or this field/byte sublist. */
diff --git a/src/extent-scan.c b/src/extent-scan.c
index e269f54..0c25c57 100644
--- a/src/extent-scan.c
+++ b/src/extent-scan.c
@@ -133,11 +133,11 @@ extent_scan_read (struct extent_scan *scan)
unsigned int i = 0;
for (i = 0; i < fiemap->fm_mapped_extents; i++)
{
- assert (fm_extents[i].fe_logical <=
- OFF_T_MAX - fm_extents[i].fe_length);
+ assert (fm_extents[i].fe_logical
+ <= OFF_T_MAX - fm_extents[i].fe_length);
- if (si && last_ei->ext_flags ==
- (fm_extents[i].fe_flags & ~FIEMAP_EXTENT_LAST)
+ if (si && last_ei->ext_flags
+ == (fm_extents[i].fe_flags & ~FIEMAP_EXTENT_LAST)
&& (last_ei->ext_logical + last_ei->ext_length
== fm_extents[i].fe_logical))
{
@@ -147,8 +147,8 @@ extent_scan_read (struct extent_scan *scan)
last_ei->ext_flags = fm_extents[i].fe_flags;
}
else if ((si == 0 && scan->scan_start > fm_extents[i].fe_logical)
- || (si && last_ei->ext_logical + last_ei->ext_length >
- fm_extents[i].fe_logical))
+ || (si && (last_ei->ext_logical + last_ei->ext_length
+ > fm_extents[i].fe_logical)))
{
/* BTRFS before 2.6.38 could return overlapping extents
for sparse files. We adjust the returned extents
diff --git a/src/id.c b/src/id.c
index 158715e..41ae024 100644
--- a/src/id.c
+++ b/src/id.c
@@ -191,8 +191,8 @@ main (int argc, char **argv)
invalid value that will be not displayed in print_full_info(). */
if (selinux_enabled
&& n_ids == 0
- && (just_context ||
- (default_format && ! getenv ("POSIXLY_CORRECT"))))
+ && (just_context
+ || (default_format && ! getenv ("POSIXLY_CORRECT"))))
{
/* Report failure only if --context (-Z) was explicitly requested. */
if (getcon (&context) && just_context)
diff --git a/src/install.c b/src/install.c
index 5468d26..854436a 100644
--- a/src/install.c
+++ b/src/install.c
@@ -359,8 +359,8 @@ setdefaultfilecon (char const *file)
/* If there's an error determining the context, or it has none,
return to allow default context */
- if ((matchpathcon (file, st.st_mode, &scontext) != 0) ||
- STREQ (scontext, "<<none>>"))
+ if ((matchpathcon (file, st.st_mode, &scontext) != 0)
+ || STREQ (scontext, "<<none>>"))
{
if (scontext != NULL)
freecon (scontext);
diff --git a/src/join.c b/src/join.c
index b92c1f8..e39ed87 100644
--- a/src/join.c
+++ b/src/join.c
@@ -173,7 +173,7 @@ static struct line uni_blank;
/* If nonzero, ignore case when comparing join fields. */
static bool ignore_case;
-/* If nonzero, treat the first line of each file as column headers -
+/* If nonzero, treat the first line of each file as column headers --
join them without checking for ordering */
static bool join_header_lines;
diff --git a/src/pr.c b/src/pr.c
index f1e310f..2ea586d 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -781,9 +781,9 @@ cols_ready_to_print (void)
n = 0;
for (q = column_vector, i = 0; i < columns; ++q, ++i)
- if (q->status == OPEN ||
- q->status == FF_FOUND || /* With -b: To print a header only */
- (storing_columns && q->lines_stored > 0 && q->lines_to_print > 0))
+ if (q->status == OPEN
+ || q->status == FF_FOUND /* With -b: To print a header only */
+ || (storing_columns && q->lines_stored > 0 && q->lines_to_print > 0))
++n;
return n;
}
@@ -1275,13 +1275,13 @@ init_parameters (int number_of_files)
/* To allow input tab-expansion (-e sensitive) use:
if (number_separator == input_tab_char)
- number_width = chars_per_number +
- TAB_WIDTH (chars_per_input_tab, chars_per_number); */
+ number_width = chars_per_number
+ + TAB_WIDTH (chars_per_input_tab, chars_per_number); */
/* Estimate chars_per_text without any margin and keep it constant. */
if (number_separator == '\t')
- number_width = chars_per_number +
- TAB_WIDTH (chars_per_default_tab, chars_per_number);
+ number_width = (chars_per_number
+ + TAB_WIDTH (chars_per_default_tab, chars_per_number));
else
number_width = chars_per_number + 1;
@@ -1297,8 +1297,8 @@ init_parameters (int number_of_files)
power_10 = 10 * power_10;
}
- chars_per_column = (chars_per_line - chars_used_by_number -
- (columns - 1) * col_sep_length) / columns;
+ chars_per_column = (chars_per_line - chars_used_by_number
+ - (columns - 1) * col_sep_length) / columns;
if (chars_per_column < 1)
error (EXIT_FAILURE, 0, _("page width too narrow"));
@@ -1836,8 +1836,8 @@ print_page (void)
{
if (empty_line)
align_empty_cols = true;
- else if (p->status == CLOSED ||
- (p->status == ON_HOLD && FF_only))
+ else if (p->status == CLOSED
+ || (p->status == ON_HOLD && FF_only))
align_column (p);
}
}
diff --git a/src/stty.c b/src/stty.c
index 8e36593..eb07f85 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -1344,8 +1344,8 @@ set_window_size (int rows, int cols, char const
*device_name)
it's almost certainly a "struct winsize" instead.
At any rate, the bug manifests itself when ws_row == 0; the symptom is
- that ws_row is set to ws_col, and ws_col is set to (ws_xpixel<<16) +
- ws_ypixel. Since GNU stty sets rows and columns separately, this bug
+ that ws_row is set to ws_col, and ws_col is set to (ws_xpixel<<16)
+ + ws_ypixel. Since GNU stty sets rows and columns separately, this bug
caused "stty rows 0 cols 0" to set rows to cols and cols to 0, while
"stty cols 0 rows 0" would do the right thing. On a little-endian
machine like the sun386i, the problem is the same, but for ws_col == 0.
diff --git a/src/test.c b/src/test.c
index 2d562d9..31a6420 100644
--- a/src/test.c
+++ b/src/test.c
@@ -367,8 +367,8 @@ binary_operator (bool l_is_l)
test_syntax_error (_("unknown binary operator"), argv[op]);
}
- if (argv[op][0] == '=' && (!argv[op][1] ||
- ((argv[op][1] == '=') && !argv[op][2])))
+ if (argv[op][0] == '='
+ && (!argv[op][1] || ((argv[op][1] == '=') && !argv[op][2])))
{
bool value = STREQ (argv[pos], argv[pos + 2]);
pos += 3;
diff --git a/src/wc.c b/src/wc.c
index 1b8a7a4..5017377 100644
--- a/src/wc.c
+++ b/src/wc.c
@@ -292,7 +292,7 @@ wc (int fd, char const *file_x, struct fstatus *fstatus)
/* Back-up the state before each multibyte character conversion and
move the last incomplete character of the buffer to the front
of the buffer. This is needed because we don't know whether
- the 'mbrtowc' function updates the state when it returns -2, -
+ the 'mbrtowc' function updates the state when it returns -2, --
this is the ISO C 99 and glibc-2.2 behaviour - or not - amended
ANSI C, glibc-2.1 and Solaris 5.7 behaviour. We don't have an
autoconf test for this, yet. */
diff --git a/src/who.c b/src/who.c
index 84c68d7..c875b1d 100644
--- a/src/who.c
+++ b/src/who.c
@@ -590,9 +590,9 @@ scan_entries (size_t n, const STRUCT_UTMP *utmp_buf)
while (n--)
{
- if (!my_line_only ||
- strncmp (ttyname_b, utmp_buf->ut_line,
- sizeof (utmp_buf->ut_line)) == 0)
+ if (!my_line_only
+ || strncmp (ttyname_b, utmp_buf->ut_line,
+ sizeof (utmp_buf->ut_line)) == 0)
{
if (need_users && IS_USER_PROCESS (utmp_buf))
print_user (utmp_buf, boottime);
--
1.7.10.420.g9768c
>From 57e0a882ef70712f79e7255aa5c1f8ba27b4c33e Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 30 Apr 2012 10:55:18 +0200
Subject: [PATCH 2/3] maint: adjust comments to avoid FP match on
binary-operator-at-EOL
* src/ls.c (print_long_format): Reformat comment to avoid "=="
at end of line.
Also, "sortkey" is not a word: s/sortkey/sort key/.
* src/ioblksize.h: Likewise, for "|" from a shell snippet.
* src/runcon.c: Likewise, for "|" in grammar-like usage.
---
src/ioblksize.h | 4 ++--
src/ls.c | 20 ++++++++++----------
src/runcon.c | 6 +++---
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/ioblksize.h b/src/ioblksize.h
index ffd6ff3..aaea9ff 100644
--- a/src/ioblksize.h
+++ b/src/ioblksize.h
@@ -28,8 +28,8 @@
bs=$((1024*2**$i))
printf "%7s=" $bs
timeout --foreground -sINT 2 \
- dd bs=$bs if=/dev/zero of=/dev/null 2>&1 |
- sed -n 's/.* \([0-9.]* [GM]B\/s\)/\1/p'
+ dd bs=$bs if=/dev/zero of=/dev/null 2>&1 \
+ | sed -n 's/.* \([0-9.]* [GM]B\/s\)/\1/p'
done
With the results shown for these systems:
diff --git a/src/ls.c b/src/ls.c
index 800f813..397e4ea 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3443,13 +3443,13 @@ static int rev_xstrcoll_df_version (V a, V b)
{ DIRFIRST_CHECK (a, b); return cmp_version (b, a); }
-/* We have 2^3 different variants for each sortkey function
+/* We have 2^3 different variants for each sort-key function
(for 3 independent sort modes).
The function pointers stored in this array must be dereferenced as:
sort_variants[sort_key][use_strcmp][reverse][dirs_first]
- Note that the order in which sortkeys are listed in the function pointer
+ Note that the order in which sort keys are listed in the function pointer
array below is defined by the order of the elements in the time_type and
sort_type enums! */
@@ -3493,14 +3493,14 @@ static qsortFunc const sort_functions[][2][2][2] =
LIST_SORTFUNCTION_VARIANTS (atime)
};
-/* The number of sortkeys is calculated as
- the number of elements in the sort_type enum (i.e. sort_numtypes) +
+/* The number of sort keys is calculated as the sum of
+ the number of elements in the sort_type enum (i.e. sort_numtypes)
the number of elements in the time_type enum (i.e. time_numtypes) - 1
This is because when sort_type==sort_time, we have up to
- time_numtypes possible sortkeys.
+ time_numtypes possible sort keys.
This line verifies at compile-time that the array of sort functions has been
- initialized for all possible sortkeys. */
+ initialized for all possible sort keys. */
verify (ARRAY_CARDINALITY (sort_functions)
== sort_numtypes + time_numtypes - 1 );
@@ -3918,10 +3918,10 @@ print_long_format (const struct fileinfo *f)
gettime (¤t_time);
}
- /* Consider a time to be recent if it is within the past six
- months. A Gregorian year has 365.2425 * 24 * 60 * 60 ==
- 31556952 seconds on the average. Write this value as an
- integer constant to avoid floating point hassles. */
+ /* Consider a time to be recent if it is within the past six months.
+ A Gregorian year has 365.2425 * 24 * 60 * 60 == 31556952 seconds
+ on the average. Write this value as an integer constant to
+ avoid floating point hassles. */
six_months_ago.tv_sec = current_time.tv_sec - 31556952 / 2;
six_months_ago.tv_nsec = current_time.tv_nsec;
diff --git a/src/runcon.c b/src/runcon.c
index 29bf0e5..875441f 100644
--- a/src/runcon.c
+++ b/src/runcon.c
@@ -15,9 +15,9 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
/*
- * runcon [ context |
- * ( [ -c ] [ -r role ] [-t type] [ -u user ] [ -l levelrange ] )
- * command [arg1 [arg2 ...] ]
+ * runcon [ context
+ * | ( [ -c ] [ -r role ] [-t type] [ -u user ] [ -l levelrange ] )
+ * command [arg1 [arg2 ...] ]
*
* attempt to run the specified command with the specified context.
*
--
1.7.10.420.g9768c
>From e744f4b7b7de9b76fdd314aa406e9fb150b9168d Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 30 Apr 2012 10:37:14 +0200
Subject: [PATCH 3/3] maint: prohibit an operator at end of line
Many coding standards, including GNU's, advocate that when
splitting a line near a binary operator, one should put the
operator at the beginning of the continued line, rather than
at the end of the preceding one. This is for readability:
such operators are relatively important to readability, and
they are more apparent at the beginning of a line than
at the varying-column end of line,
* cfg.mk (sc_prohibit_operator_at_end_of_line): New rule.
Exempt test.c and head.c.
---
cfg.mk | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index c3ddb42..923785e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -200,6 +200,18 @@ sc_no_exec_perl_coreutils:
exit 1; } || :; \
fi
+# With split lines, don't leave an operator at end of line.
+# Instead, put it on the following line, where it is more apparent.
+# Don't bother checking for "*" at end of line, since it provokes
+# far too many false positives, matching constructs like "TYPE *".
+# Similarly, omit "=" (initializers).
+binop_re_ ?= [-/+^!<>]|[-/+*^!<>=]=|&&?|\|\|?|<<=?|>>=?
+sc_prohibit_operator_at_end_of_line:
+ @prohibit='. ($(binop_re_))$$' \
+ in_vc_files='\.[chly]$$' \
+ halt='found operator at end of line' \
+ $(_sc_search_regexp)
+
# Don't use "readlink" or "readlinkat" directly
sc_prohibit_readlink:
@prohibit='\<readlink(at)? \(' \
@@ -456,3 +468,7 @@
exclude_file_name_regexp--sc_prohibit_continued_string_alpha_in_column_1 = \
exclude_file_name_regexp--sc_prohibit_test_backticks = \
^tests/(init\.sh|check\.mk|misc/stdbuf)$$
+
+# Exempt test.c, since it's nominally shared, and relatively static.
+exclude_file_name_regexp--sc_prohibit_operator_at_end_of_line = \
+ ^src/(ptx|test|head)\.c$$
--
1.7.10.420.g9768c
- Re: [PATCH 3/3] maint: prohibit an operator at end of line,
Jim Meyering <=