7

I'm currently writing some code for embedded systems (both in c and c++) and in trying to minimize memory use I've noticed that I used a lot of code that relies on integer promotions. For example (to my knowledge this code is identical in c and c++):

uint8_t brightness = 40;
uint8_t maxval = 255;
uint8_t localoutput = (brightness * maxval) / 100;

So even though brightness * 255 is larger than what can be stored in an uint8_t, this still yields the correct result due to, if I'm correct, integer promotions. Brightness is a percentage so it should never be higher than 100 and therefore localoutput should never be higher than 255. My question is then whether or not any unexpected behaviour (such as brightness * maxval being larger than 255 therefore having overflow) or any significant differences between how this syntax is handled between c++ and c are the case. It seems to just output the correct answer, or would be more recommended to have the variables be of type uint16_t as the intermediate calculations may be higher than 255, and just take the memory loss for granted.

TomK
  • 47
  • 6
  • 1
    This other post has some info about promotion, which is not your case I believe. https://stackoverflow.com/questions/44455248/integer-promotion-in-c .But as far as your calculations go, I would say it looks fine since it doesn't cross the representable maximum value of 255. Also, best practice is to use the very least allocation of resources you need and not having an uint16_t that stores a value that an uint8_t also can. – iSpark Mar 03 '22 at 16:48
  • 1
    If you want to make the code explicit use `static_cast` in the expression. Note that `100` is already an `int`. – Richard Critten Mar 03 '22 at 16:49
  • 3
    Qualitative evaluations such as "bad practice" are often primarily a matter of opinion. Lots of us are pretty opinionated, but questions that are primarily about matters of opinion are off topic here. – John Bollinger Mar 03 '22 at 16:49
  • 2
    And it only gets worse when you spam tags, such as by tagging both [c] and [c++]. Opinions often differ more sharply across different areas of practice such as those. – John Bollinger Mar 03 '22 at 16:51
  • 2
    You can't ask important big picture questions like this on SO. I'd recommend using https://software.codidact.com/ instead, which is more tolerant to design questions. You would still have to settle for one single language at a time though, not two at once. – Lundin Mar 03 '22 at 16:59
  • 1
    I disagree with the closure. The question can be answered factually by laying out the various technical factors to consider without declaring one answer to be right. Answerers can explain what the tradeoffs are and leave the conclusion-drawing up to the readers. – John Kugelman Mar 03 '22 at 17:01
  • 1
    @JohnKugelman Be that as it may, it is too broad with both language tags. We shouldn't reopen it until that is fixed. – Lundin Mar 03 '22 at 17:10
  • 1
    [[tag:c]] Is it safe? Yes. An integer's minimum range can accommodate 0..25500. Is it a good practice? Well, that's an opinion. But it's not misleading. It does exactly what one would expect it to do at a glance. So I don't see how someone could consider it a bad practice. – ikegami Mar 03 '22 at 17:21
  • 1
    @JohnKugelman You can say what happens when you use integer promotion and let the readers draw a conclusion... but that's not answering the question. The question asks: "is it bad?". – Passer By Mar 03 '22 at 17:26
  • 1
    @ikegami All it takes is `(brightness * maxval)<<1` and bam, it is neither safe nor good practice, but explicitly UB on all 16 bit systems. If someone thinks the shift is carried out on 8 bit types, they could easily write such bugs. – Lundin Mar 03 '22 at 17:30
  • 1
    @Lundin, 1) I didn't comment on `(brightness * maxval)<<1`, and 2) that would fail with `((int)brightness * (int)maxval)<<1` too, so I don't get your point. – ikegami Mar 03 '22 at 17:32
  • 1
    @ikegami The point is that the operands were both unsigned until C's implicit promotion rules silently changed the signedness. – Lundin Mar 03 '22 at 17:38
  • 1
    @Lundin The question was about implicit vs explicit casts. Your example doesn't contribute to that. Furthermore, your example doesn't even demonstrate what you claim it does. It would fail even if signedness didn't change. (It wouldn't be UB, but it wouldn't give the right result either on a 16-bit system.) – ikegami Mar 03 '22 at 17:41
  • 1
    @ikegami The question doesn't mention casts and there is no such thing as an implicit casts so I don't know what you are talking about. – Lundin Mar 03 '22 at 17:42
  • 1
    @Lundin, Yes it does mention casts Specifically, integer promotions, a form of implicit cast. Ok, the alternative was using a larger variable type rather than a cast ...which is weird question since integer promotion would still occur X_X. The point stands, though. I still have no clue why you posted your comment and why you tagged me. If you applied what I said to your code, you clearly get a different outcome. An integer's minimum range *can't* accommodate 0..100*255*2. So that *wouldn't* be safe. And it *would* be misleading. So it *would* be a bad practice. – ikegami Mar 03 '22 at 17:48

1 Answers1

3

Your question raises an important issue in C programming and in programming in general: does the program behave as expected in all cases?

The expression (brightness * maxval) / 100 computes an intermediary value brightness * maxval that may exceed the range of the type used to compute it. In Python and some other languages, this is not an issue because integers do not have a restricted range, but in C, C++, java, javascript and many other languages, integer types have a fixed number of bits so the multiplication can exceed this range.

It is the programmer's responsibility to ascertain that the range of the operands ensures that the multiplication does not overflow. This requires a good understanding of the integer promotion and conversion rules, which vary from one language to another and are somewhat tricky in C, especially with operands mixing signed and unsigned types.

In your particular case, both brightness and maxval have a type smaller than int so they are promoted to int with the same value and the multiplication produces an int value. If brightness is a percentage in the range 0 to 100, the result is in the range 0 to 25500, which the C Standard guarantees to be in the range of type int, and dividing this number by 100 produces a value in the range 0 to 100, in the range of int, and also in the range of the destination type uint8_t, so the operation is fully defined.

Whether this process should be documented in a comment or verified with debugging assertions is a matter of local coding rules. Changing the order of the operands to maxval * brightness / 100 and possibly using more explicit values and variable names might help the reader:

uint8_t brightness100 = 40;
uint8_t localoutput = 255 * brightness100 / 100;

The problem is more general than just a question of integer promotions, all such computations should be analyzed for corner cases and value ranges. Automated tools can help perform range analysis and optimizing compilers do it to improve code generation, but it is a difficult problem.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 1
    Wouldn't `brightness100/100` be truncated to 0 if the variable is always less than 100? – Hong Ooi Mar 31 '22 at 19:55
  • 1
    @HongOoi: multiplication and division have the same precedence and associate left to right, hence `255 * brightness100 / 100` is parsed as `(255 * brightness100) / 100`. – chqrlie Mar 31 '22 at 19:59