Loop does not set value correctly

Question

Why does the loop below not set the pinMode to OUTPUT for the pins in the array pins[]?

int pins[21];

void setup() {

    int pins[] = {13,
              3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
              14, 15, 16, 17, 18, 19, 20, 21, 22, 23};

    for (int i = 0; i < sizeof(pins); i++) {
        pinMode(pins[i], OUTPUT);
    }

}

To get it to work, I have to set them manually:

pinMode(3, OUTPUT);    
pinMode(4, OUTPUT);
pinMode(5, OUTPUT);
// ...

If pinMode takes an index and not a reference, then you should give it i and not pins[i] inside the loop. Instead you are just setting the indexes that happen to be values inside the array... — Veltas, Sep 20 '13 at 19:12
This could be something on the Arduino that I'm not familiar with, but you seem to be setting `pinValues[i] = LOW`, but setting the pin mode for values passed in the `pins` array. Do you mean to set `pinValues[pins[i]] = LOW`? — Zac Howland, Sep 20 '13 at 19:14
@ZacHowland I meant to store the `LOW` value in the array `pinValues[]`. However using `sizeof(pins)/sizeof(pins[1])` for the loop definition does not seem to set `pinMode()` to `OUTPUT` — Nyxynyx, Sep 20 '13 at 19:16
Read [Weird behavior when printing array in C?](http://stackoverflow.com/questions/18009725/weird-behavior-when-printing-array-in-c?answertab=votes#tab-top) — Grijesh Chauhan, Sep 20 '13 at 19:17

score 2 · Accepted Answer · edited May 23 '17 at 11:56

Wrong breaking condition causes Undefined behaviour due to n index-out-of bound problem :

for (int i = 0; i < sizeof(pins); i++)
                    ^-----------^

should be:

for (int i = 0; i < sizeof(pins)/sizeof(pins[0]); i++)
                    ^---------------------------^

Note: When you apply the sizeof operator to an array type, the result is the total number of bytes in the array.

To understand formula read: Weird behavior when printing array in C?

score 1 · Answer 2 · answered Sep 20 '13 at 19:08

1

Change sizeof(pins) to sizeof pins / sizeof *pins.

sizeof yields the size of the array in bytes and not the number of elements of the array.

answered Sep 20 '13 at 19:08

ouah

142,963
15
272
331

@Veltas `sizeof *pins` is `sizeof (int)` here. Using the object name has the advantage you don't need to change the type in the `sizeof` operand if you change the type of array. – ouah Sep 20 '13 at 19:11
Oh God I read that as sizeof pins*, not sizeof *pins. Don't worry! – Veltas Sep 20 '13 at 19:14

score 0 · Answer 3 · edited Sep 20 '13 at 19:23

0

For starters, sizeof(pins) will be sizeof(int) times the number of entries in pins. Accessing values outside the range causes undefined behavior. You probably meant to use

sizeof(pins)/sizeof(pins[0])

or in C++ just size(pins) with a suitable function template like

template <typename T, std::size_t Size>
constexpr std::size_t size(T(&)[Size]) { return Size; }

Why the functions don't work can't be seen from the code snippet you provided as the code doesn't include the definition of the functions (or macros) pinMode().

edited Sep 20 '13 at 19:23

Zac Howland

15,777
1
26
42

answered Sep 20 '13 at 19:14

Dietmar Kühl

150,225
13
225
380

Why did you choose to use index `1` rather than `0` in `izeof(pins)/sizeof(pins[1])`? – Nyxynyx Sep 20 '13 at 19:17
@Nyxynyx: it is a typo and I shall correct it. May be an interesting research to find out if it matters: since it is a non-evaluated context any index might do but `0` certainly works (as an array needs to have at least one element). – Dietmar Kühl Sep 20 '13 at 19:24

score 0 · Answer 4 · answered Sep 20 '13 at 19:27

Although you can use sizeof(array)/sizeof(array[0]) or a template to compute the size of the array (or, equivalently, use it to find the end of the array), if you're using a reasonably current compiler, it already knows how to handle the job correctly on its own:

for (auto i : pins)
    pinMode(i, OUTPUT);

At least in my opinion, this seems like the simplest of the three.

Loop does not set value correctly

4 Answers4