Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

Re: [PATCH v4 38/44] builtin-am: support and auto-detect StGit patches

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:05:35

Eric Sunshine [off-list ref] writes:
On Mon, Jun 29, 2015 at 4:42 PM, Stefan Beller [off-list ref] wrote:
quoted
On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan [off-list ref] wrote:
quoted
+/**
+ * Returns true if `str` consists of only whitespace, false otherwise.
+ */
+static int str_isspace(const char *str)
+{
+       while (*str)
+               if (!isspace(*(str)++))
+                       return 0;
...
    while (*str && !isspace(*(str)++))
        return 0;
...
Ugh. Please don't break the logic with this strange and bogus transformation.

If you really want it to read more idiomatically, try:

    for (; *s; s++)
        if (!isspace(*s))
            return 0;
;-).

Regardless of the loop structure, I find

	*(str)++

especially ugly and confusing.  I'd understand if it were

	*(str++)

but the parentheses pair is unnecessary.

Not using any increment inside isspace(), like you showed, is the
most readable.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help