0

This is an exercise from the book C Programming: A modern approach by K.N King

"Assume that the following array contains a week's worth of hourly temperature readings, with each row containing the readings for one day: int temperatures[7][24]; Write a statement that uses the search function to search the entire temperatures array for the value 32."

My code below includes my search function from a previous exercise, and main() where I call the function. Search simply processes an array and checks if any element is equal to a supplied argument "key". The solution appears to compile and execute the expected output, it prints "32 was found", or prints nothing if I comment out the assignment temperatures[5][22] = 32;.

Notice how the function takes a 1d array but the exercise is asking to process a 2d array.

I initially tried this solution without the explicit type cast (int*) in the function call, and got this when compiling (I reformatted it a bit):

1 compiler warning: "passing argument 1 of search from incompatible pointer type"

1 note: "expected 'const int *' but argument is of type 'int * [24]'"

Why do the warnings/note appear? If there is an incompatibility, why are they not errors? The type cast to int* removes any compiling issues, but is the solution actually correct/safe or bad C understanding/practice on my part? I understand that the function expects int* but that the 2d array decays to a pointer of type int * [24] without the type cast. Yet the code works in either case, albeit I just had one test case. How could I modify my solution logic to avoid this issue at all? The search function should stay the same though.

#include <stdbool.h>
#include <stdio.h>

bool search(const int a[], int n, int key);

int main(void){

    int temperatures[7][24] = {0};
    temperatures[5][22] = 32;
    bool has32 = search((int*)temperatures, 7 * 24, 32);

    if(has32)
        printf("32 was found");
    return 0;
}

bool search(const int a[], int n, int key){

    const int *p;

    for(p = a; p < a + n; p++)
        if(*p == key)
            return true;
    return false;
}
Allan Wind
  • 23,068
  • 5
  • 28
  • 38
0jnats3
  • 85
  • 1
  • 7
  • Is there really not a canonical duplicate for this? – Karl Knechtel Jan 10 '23 at 05:35
  • Does this answer your question? [Map a 2D array onto a 1D array](https://stackoverflow.com/questions/2151084/map-a-2d-array-onto-a-1d-array) – Allan Wind Jan 10 '23 at 05:59
  • Generally, you can't without invoking some kind of *undefined behavior*. The closest thing is using `(int*)&temperatures` but the C standard is quite ambiguous about on this topic. Taking the C standard literally will make pretty much every use of `malloc`-ed memory undefined. – tstanisl Jan 10 '23 at 09:32

4 Answers4

5

Pass the address of the first element but it's technically undefined behavior to index the array past column index 23:

bool has32 = search(&temperatures[0][0], 7 * 24, 32);

The better option is to change the prototype of search() to use a VLA:

bool search(size_t rows, size_t cols, const int a[rows][cols], int key) {
    for(size_t r = 0; r < rows; r++)
        for(size_t c = 0; c < cols; c++)
            if(a[r][c] == key)
               return true;
    return false;
}

and call it like this:

    bool has32 = search(
        sizeof(temperatures) / sizeof(*temperatures),
        sizeof(*temperatures) / sizeof(**temperatures),
        temperatures,
        32
    );
Allan Wind
  • 23,068
  • 5
  • 28
  • 38
  • It makes total sense and I feel like a fool for not realizing it was that simple at first. However, could you elaborate a bit on my questions about the type casting and the compiler and the correctness of my supplied solution? For example, you just edited your post to mention it is technically undefined behavior. Why does the compiler not produce an error when it encounters undefined behavior? – 0jnats3 Jan 10 '23 at 05:32
  • You are casting `int temperatures[7][24]` is to a `(int *)` which are different types. The other one is easy your cast is `(int *)` but you want `(const int *)`. – Allan Wind Jan 10 '23 at 05:38
  • just to reiterate, with the cast, I do not get any errors. Does such a cast actually change anything except for that? Do I lose information? Get undefined behavior? Or is this an acceptable solution is what i'm trying to understand. Your solution also works, without compiler errors. Is there any objective difference between the two? – 0jnats3 Jan 10 '23 at 05:44
  • 1
    @0jnats3 You do lose information about the dimension of the array when casting it an (int *) unlike the VLA solution that I provided. – Allan Wind Jan 10 '23 at 05:59
  • the question has been answered, by you and another poster with different solutions. Thanks for explaining that I do indeed lose information, the dimensions of the array thrown out the window when I do the type cast. – 0jnats3 Jan 10 '23 at 06:02
  • 1
    @0jnats3 When you cast you invoke undefined behavior, strictly speaking. I posted yet another answer with some references. – Lundin Jan 10 '23 at 09:12
  • @Lundin, the cast is not an issue. The problem is dereferencing the pointer later on. – tstanisl Jan 10 '23 at 11:26
  • @tstanisl ...which wouldn't be possible if not for the dirty cast and ignoring the compiler messages about invalid C being written... – Lundin Jan 10 '23 at 12:08
1

If you can change the search() function parameter type, change it to receive 2D array.
If you cannot then call the search() in a loop and in each iteration pass the element of 2D array temperatures to search(). The elements of 2D array temperatures are 1D array of type int [24] which can be passed to search().

In main() you should do:

    bool has32 = 0;
    for (int i = 0; i < 7; ++i) {
        has32 = search(temperatures[i], 24, 32);
        if (has32) break;
    }

This is perfectly compatible and will not invoke any kind of UB.

Also, instead of using numbers (like 7 and 24), may you give some name to them for better readability, like this

#define ROWS    7
#define COLUMNS 24

Use them in you program when defining array temperatures:

    int temperatures[ROWS][COLUMNS] = {0};
    int key = 32;

    temperatures[5][22] = key;

    bool has32 = 0;
    for (int i = 0; i < ROWS; ++i) {
        has32 = search(temperatures[i], COLUMNS, key);
        if (has32) break;
    }
H.S.
  • 11,654
  • 2
  • 15
  • 32
  • This is how I envisioned the solution but could not write it down my self. By passing "one dimension" of the "two dimension array" in a loop in main. Thank you very much. But by looking at my code, if you were a teacher, would you accept the type cast I did, or would you comment on it, and if so, what would you say? (either way, thx for the proper solution given the constraints). – 0jnats3 Jan 10 '23 at 05:50
  • 1
    Use a sizeof instead of hard-coding the 7 and 22 (see my answer; possible wrap it in a macro). – Allan Wind Jan 10 '23 at 05:55
  • 1
    @Ojnats3 You program is not strictly conforming program if you alias a `2D` array to a `1D` array. – H.S. Jan 10 '23 at 06:02
  • While this may be strictly conforming, it's not really ideal to iterate through one dimension of the array at a time. Apart from the the function call overhead, it might not be ideal for cache performance either. The most proper fix to a bad function API is to rewrite the function, if that's an option. – Lundin Jan 10 '23 at 09:09
  • @Lundin I do agree with you.. and this is what the first line of my post is suggesting and rest of answer comes after _If you cannot then ....._. – H.S. Jan 10 '23 at 09:25
1

First of all please check out this good, recent question: Is a two-dimensional array implemented as a continuous one-dimensional array? The answer by @dbush shows what the C language allows and doesn't allow in terms of mixing 1D arrays and 2D arrays. The answer by yours sincerely shows some possible work-arounds. You have to use one such work-around or you will be invoking undefined behavior (and the book you are reading ought to tell you as much unless it's crap).


I initially tried this solution without the explicit type cast (int*) in the function call, and got this when compiling (I reformatted it a bit):

1 compiler warning: "passing argument 1 of search from incompatible pointer type"

Because you can't wildly cast to int. A int* is not compatible with an int(*)[n], the code is a constraint violation, meaning it is not valid C. And you can't use an int* to iterate through a 2D array anyway.


Why do the warnings/note appear? If there is an incompatibility, why are they not errors?

Because the C language defines no such thing as "warnings/errors", it's a de facto standard invented by compilers. The C language only requires that some manner of diagnostic message is given when you feed the compiler invalid C. See What must a C compiler do when it finds an error?


but is the solution actually correct/safe or bad C understanding/practice on my part?

It is bad C as explained by the first link given in this answer.


Yet the code works in either case

Compilers tend to implement this manner of type punning in well-defined ways internally, but formally you are invoking undefined behavior. "It seems to work just fine" is one form of undefined behavior.


How could I modify my solution logic to avoid this issue at all? The search function should stay the same though.

As others have mentioned, the best way is to rewrite the function to use a 2D array instead. If that's not an option, you can use the union type punning example from my answer in the link at the top, then pass the union "arr_1d" member to the function.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • It would however be enlightening to research what your book seems to think is the correct answer, since that would say a lot about the quality of the mentioned book. In case the author is aware of the array out of bounds issue and addresses it, then it's a good remark. If they aren't aware of it but teach casting to `int*`, then it's a bad book teaching undefined behavior. After such research we can either label the author either guru or quack and recommend/dismiss the book accordingly. – Lundin Jan 10 '23 at 08:41
  • The explicit cast from `int(*)[n]` to `int*` is itself fine. C standard tells "A pointer to an object type may be converted to a pointer to a different object type." as long as alignment requirements are satisfied what is true for any n-dimensional array of `int`. – tstanisl Jan 10 '23 at 11:32
  • @tstanisl No, the cast is not "fine", it is just hiding problems. If you are out driving and come to a crossing with red light, nothing in the car will prevent you from driving against red and into a car crash. But if you do, it is pretty strange to then blame the cars for breaking when hitting each other and stating that as the source of the traffic accident. The source was you driving against red even when explicitly told not to. – Lundin Jan 10 '23 at 12:15
  • No. The cast itself is ugly, but it is perfectly defined by C standard. When answering the question please separate conclusions based on the C standard itself from ones based your own aesthetic taste (which is IMO justified anyway). Your "red light" analogy does not apply at all to this case. – tstanisl Jan 10 '23 at 12:21
  • @tstanisl I know perfectly well that C allows all manner of wild and crazy (object) pointer conversions. So the cast in itself isn't poorly defined, but it is still wrong. The compiler tells you "stop, this is not valid C". If you then chose to cast then you tell the compiler "shut up, I (don't) know what I'm doing", resulting in undefined behavior later on. – Lundin Jan 10 '23 at 13:30
1

This is alternative answer to a better solution.

Use a union of 2D and 1D array. The C standard allows inspecting one member via the other member:

union {
  int temperatures[7][24];
  int as_1d[7 * 24];
} u = { 0 };
u.temperatures[5][22] = 32;
bool has32 = search(u.as_1d, 7 * 24, 32);
tstanisl
  • 13,520
  • 2
  • 25
  • 40