On 09/11/2017 08:58 AM, John Marshall wrote:
> On 11 Sep 2017, at 11:40, Kamil Dudka via curl-library <curl-library_at_cool.haxx.se> wrote:
>> While I respect your opinion, I disagree on this. Not using ternary operator
>> leads to code duplication, which some programmers, including me, try to avoid.
>
> Agreed, the now-duplicated assignment is less clear to me -- I have to stop and check whether the LHSes are the same. To micro-bikeshed this further, perhaps the following version avoids Ben's warning while being arguably simpler than the original (as it shows the possible code path combinations directly):
>
> diff --git a/lib/mime.c b/lib/mime.c
> index 74b653649..b26f914d5 100644
> --- a/lib/mime.c
> +++ b/lib/mime.c
> @@ -296,9 +296,12 @@ static char *escape_string(const char *src, size_t len)
> for(i = 0; len; len--) {
> char c = *src++;
>
> - if(c == '"' || c == '\\' || !c)
> - dst[i++] = '\\';
> - dst[i++] = c? c: '0';
> + if(c == '"' || c == '\\')
> + dst[i++] = '\\', dst[i++] = c;
> + else if(c == '\0')
> + dst[i++] = '\\', dst[i++] = '0';
> + else
> + dst[i++] = c;
> }
>
> dst[i] = '\0';
>
> Adjust the comma operators to { ; } instead as taste dictates...

This compiles w/out warnings on my Fedora-14 machine. I added
braces and replaced the , with semi colons.

[greearb_at_v-f14-64 curl-git]$ git diff
diff --git a/lib/mime.c b/lib/mime.c
index 5ed8849..36c83ad 100644
--- a/lib/mime.c
+++ b/lib/mime.c
@@ -295,14 +295,18 @@ static char *escape_string(const char *src, size_t len)

    for(i = 0; len; len--) {
      char c = *src++;
-
- if(c == '"' || c == '\\' || !c)
+ if(c == '"' || c == '\\') {
        dst[i++] = '\\';
- if(c)
        dst[i++] = c;
- else
+ }
+ else if(c == '\0') {
+ dst[i++] = '\\';
        dst[i++] = '0';
- }
+ }
+ else {
+ dst[i++] = c;
+ }
+}

    dst[i] = '\0';
    return dst;

Thanks,
Ben

>
> John
> -------------------------------------------------------------------
> Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
> Etiquette: https://curl.haxx.se/mail/etiquette.html
>

-- 
Ben Greear <greearb_at_candelatech.com>
Candela Technologies Inc  http://www.candelatech.com
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html