Getting unexpected results for array of strings when using = operator

Question

In the below code I am getting unexpected result. During first iteration s[0] is getting printed as the first element. But during the remaining iteration, 0 is getting printed. Is it wrong to use = operator instead of strcpy? I tried using strcpy but it results in segmentation fault. Why is wrong value getting printed?

int main() {
    int n, i;
    scanf("%d", &n);
    int c, j;
    char* s[n];
    char* ch = (char*)malloc(100);
    for (i = 0; i < n; i++) {
        scanf("%s", ch);
        s[i] = strdup(ch);
    }

    char* final[n];
    int count[n];
    for (i = 0; i < n; i++) {
        final[i] = "";
        count[i] = 0;
    }
    int end;
    for (i = 0; i < n; i++) {
        end = 0;
        while (1) {
            printf("%s\n", s[0]);
            if (strcmp(final[end], "")) {
                count[end]++;
                final[end] = s[i];
                // printf("%s\n",final[end]);
                end++;
                break;
            }
            else if (strcmp(final[end], s[i])) {
                final[end] = s[i];
                sprintf(final[end], "%d", count[end]);
                count[end]++;
                end++;
                break;
            }
            end++;
        }
    }
    for (i = 0; i < n; i++) {
        // printf("%s\n",final[i]);
    }
    return 1;
}

[Please see this discussion on why not to cast the return value of malloc() and family in C..](https://stackoverflow.com/q/605845/2173917) — Sourav Ghosh, Jun 21 '17 at 06:24
"why not to cast" - and keep an eye, too, on [this](https://stackoverflow.com/a/14879184/1312382) answer. Truth is, that there are two opposing fractions, one in favour and one against casting. So decide yourself what is more natural for you... — Aconcagua, Jun 21 '17 at 06:42
@Aconcagua "the other fraction" is typically C++ programmers, who also prefer to write `int* a` instead of `int *a`. There's nothing wrong with making intention explicit of course, but in case of `void *`, an assignment is explicit enough for a *C* programmer. — , Jun 21 '17 at 07:03
@Aconcagua adding to my comment, it's a good thing to present counter-arguments to something not universally agreed on. I just think it has to do with a *C mindset* vs. *C++ mindset*. To support this, consider that casting of pointers is common in C++ (because of inheritance), but often a sign for undefined behavior in C (you should use unions of structs with a common initial sequence here). A good C program often doesn't have a single cast between pointer types. — , Jun 21 '17 at 07:13
@FelixPalmen C++ offers more fine grained casting via static_cast, dynamic_cast, const_cast, reinterpret_cast, which should be preferred over the classic C style cast. A reinterpret_cast would correspond to the cast in C in context of UB you mention. However, there are quite a lot of counter examples in pure C, such as socket programming (`(sockaddr*)&sockaddr_in`). Another counter example: a header-only library (probably not that a good idea if one needs malloc there...), still, it would get unusable from C++ if `malloc`ing without cast (sure, some other stuff such as VLA to be avoided, too). — Aconcagua, Jun 21 '17 at 07:28
@Aconcagua I'm quite aware of the C++ cast operators, I think they somewhat support my argument above? The classical socket API is btw a mess and actually an example for bad C code. — , Jun 21 '17 at 07:34
Not sure - "A good C program often doesn't have a single cast" is not true - an implilcit cast is still a cast... Same argument would then be applicable to skipping any unnecessary parentheses (`if((x = get()))` or `if((a && b) || (c && d))`) where many compiler warn about if not set explicitly... — Aconcagua, Jun 21 '17 at 07:49
But now we are at a discussion which here is not the right place for - and I actually did not want to start. My original intention was just to oppose trying to push people into one or another direction ("you should not") - there are two oppinions and everybody should be free to make up his/her own mind, just as I would have opposed to if anyone would have tried to force others into k&r style (yeah, I consider allman superior...). — Aconcagua, Jun 21 '17 at 07:56
@Aconcagua: An "implicit cast" is actually called an "implicit conversion", so by the wording of the standard, it isn't a cast. The conversion between pointer types is *only* implicit to and from `void *`, so anything else is a code smell (note my wording included "between pointer types"). — , Jun 21 '17 at 07:56
@FelixPalmen I don't see much difference in the *argument* if I (correctly) reword into implicit conversion vs. explicit conversion (*via* cast)... I finally stay with: If people prefer to cast, just *let* them - *and* the other way round, if people prefer *not* to, just let them as well... — Aconcagua, Jun 21 '17 at 08:14
In addition to the linked duplicate, also see [How to correctly assign a new string value?](https://stackoverflow.com/questions/3131319/how-to-correctly-assign-a-new-string-value). These are found in the [SO C FAQ](https://stackoverflow.com/tags/c/info). — Lundin, Jun 21 '17 at 12:31

Ely · Accepted Answer · 2017-06-21T07:02:58.807

3

if (strcmp(final[end], ""))

strcmp returns 0 if final[end] compares equally to the empty string.
A 0 in the if condition means false, so the if-block is not executed. I believe you want it executed. So you should do

if (strcmp(final[end], "") == 0)

The same applies to the else if statement.

edited Jun 21 '17 at 07:02

answered Jun 21 '17 at 06:49

Ely

10,860
4
43
64

You mean `== 0`... – Aconcagua Jun 21 '17 at 07:00
Yes, indeed. Thanks for correcting my brain twist. – Ely Jun 21 '17 at 07:03
oh god.. That was so silly.. Thanks – user3762146 Jun 21 '17 at 10:21

Getting unexpected results for array of strings when using = operator

1 Answers1