I'm a beginner on C and I don't understand all features of this beautiful language yet. So, I have a very strange problem which doesn't affect my solution at all, I anyway get the right result.

So, the task is:

1. Given an array of integers.

2. Return an array, where the first element is the count of positives numbers and the second element is sum of negative numbers. 0 is neither positive nor negative.

3. If the input is an empty array or is null, return an empty array.

4. Example: For input [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, -11, -12, -13, -14, -15], you should return [10, -65].

My solution:

void count_positives_sum_negatives(
  int *values, size_t count, int *positivesCount, int *negativesSum) 
{
  while(count-- > 0 ? (*values > 0 ?   *positivesCount : (*negativesSum  = *values))   values   : 0);
}  

'count' contains size of array

But it gives me this "error" or "warning", which strangely doesn't affect my program at all:

solution.c:6:94: warning: unsequenced modification and access to 'values' [-Wunsequenced]
  while(count-- > 0 ? (*values > 0 ?   *positivesCount : (*negativesSum  = *values))   values   : 0); 
                        ~~~~~~                                                               ^
1 warning generated.

How do I fix this?

FIXED:

while(count-- > 0 ? (*values > 0 ?   *positivesCount : (*negativesSum  = *values))   1 : 0) values  ;

CodePudding user response：

You write (*values...) values . There's no sequence point between the operands of , so reading from (ie: using values in the expression *values) and writing to (ie: updating values in values ) is undefined behavior.

To fix, simply write the code more simply, using multiple statements and expressions rather than try to one-line it.

For example, I might write it like this:

typedef struct stats_s {
    int positive_counts;
    int64_t negative_sum;
} stats_s ;

stats_s get_stats(const int *xs, size_t count) {
    stats_s s = {0, 0};
    for (size_t i = 0; i < count; i  ) {
        if (xs[i] > 0) s.positive_counts  ;
        else s.negative_sum  = xs[i];
    }
    return s;
}

CodePudding user response：

On your question

"But it gives me this "error" or "warning", which strangely doesn't affect my program at all".

It's a warning. Usually when you get a warning, you have a problem in your code. In this case you have Undefined Behavior (UB on StackOverflow).

For your specific question ("How do I fix this?"), the answer is "don't write such monstrosity". Avoid one liners.

But, probably you want to know where the problem is. The problem is that in this sum

(*values > 0 ?   *positivesCount : (*negativesSum  = *values))   values  
  ~~~~~~                                                               ^

you can legally evaluate values before or after *values. So, depending on the compiler feeling for this, it can generate different machine code (and behavior) for the same source code. It's usually possible to observe this, by changing the optimization levels or in MSVC switching between Debug and Release mode.

What did you expect? values to happen after the first term is evaluated? Then put it in another statement.

On the problem statement

In C language you cannot "return an array". So it's impossible to fulfill the request. Your code lacks all the checks required by the problem statement. You also didn't reset the accumulator variables.

If we assume that "return an array" = "return an allocated piece of memory able to contain two numbers or NULL to indicate an empty array" (which is totally arbitrary), this could be a solution:

int *count_positives_sum_negatives(int *values, size_t count) 
{
    if (values == NULL || count == 0) {
        return NULL;
    }
    int *ret = calloc(2 * sizeof(int));
    
    for (size_t i = 0; i < count;   i) {
        if (values[i] > 0) {
            ret[0]  = 1;
        }
        else {
            ret[1]  = values[i];
        }
    }
    
    return ret;
}

CodePudding user response：

I found the solution, just move values to the body of the loop:

#include <stddef.h>

void count_positives_sum_negatives(
  int *values, size_t count, int *positivesCount, int *negativesSum) 
{
  while(count-- > 0 ? (*values > 0 ?   *positivesCount : (*negativesSum  = *values))   1 : 0) values  ;
}  

So no more undefined behaviour.