-1

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++;
Improve this question
7
  • 1
    There's no sequencing between *values and values++, so there's no way to say which operation will be evaluated first. Commented Sep 25, 2022 at 12:30
  • George Glebov, Think about (*values > 0 ? ++*positivesCount : (*negativesSum += *values)) + values++. Which must evaluate first? (*values > 0 ? ++*positivesCount : (*negativesSum += *values)) or values++? Commented Sep 25, 2022 at 12:30
  • 1
    Is this a code-golf challenge? Why make your life harder with such an insane while loop statement? Commented Sep 25, 2022 at 12:31
  • @chux-ReinstateMonica I thought it works from left to right. Ok, I'll try to somehow make it evaluate in order I want. Commented Sep 25, 2022 at 13:43
  • 1
    Is there a reason you're using ?: here, instead of a more conventional if/else statement? There are good uses for ?:, but this doesn't strike me as one of them. Commented Sep 25, 2022 at 20:26

3 Answers 3

Reset to default
2

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; }
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I found the solution, just replace value++ with 1 and move right after while loop: while(count-- > 0 ? (*values > 0 ? ++*positivesCount : (*negativesSum += *values)) + 1 : 0) values++;
1

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; }
Improve this answer

2 Comments

I just moved values++ to the body of the while loop and replaced it with 1. Perfectly works.
@GeorgeGlebov Beware that "perfectly works" is not always the same as "works for the right reasons". Sometimes, code that seems to work perfectly today, mysteriously stops working tomorrow. So there's more to programming than just "seems to work".
-1

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.

Improve this answer

5 Comments

The solution still doesn't conform to the problem statement. You are not making any check and you are not "returning an array", whatever that means.
I'm glad you got it working, and it sounds like you're happy, too, but if you were writing code in anything other than a self-learning environment like it sounds like you are, you should know that everyone else will hate you for writing cryptic code like this.
Beware that "the compiler didn't give me any warnings" is not the same as "no more undefined behavior". There are many kinds of undefined behavior that are impossible for a compiler to detect, and that you therefore won't get warnings for. Truly eliminating undefined behavior requires care and understanding on your part.
@SteveSummit by the problem statement I know that these values are set to 0 in subprogram which calls my function. And if the array is empty, my program returns 0 because count is equal to zero in this case: sizeof(values)/sizeof(values[0]). So count-- > 0 immediately returns 0 so positivesCount and negativeSum don't change - they keep 0.
@GeorgeGlebov If you have a problem statement, then you have a contract which must be fulfilled between your function and the calling software. The problem statement requires you to "return an array" not two variables. Moreover you must "return an empty array" if the pointer is NULL. Your code just accesses the pointer and happily segfaults.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.