Path: csiph.com!eternal-september.org!feeder.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail From: Keith Thompson Newsgroups: comp.lang.c Subject: Re: Understanding the program that has pointers to two_dimensional arrays Date: Mon, 05 Dec 2016 09:26:56 -0800 Organization: None to speak of Lines: 285 Message-ID: References: <481da833-61f6-4f1c-973b-1b129118c682@googlegroups.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: mx02.eternal-september.org; posting-host="f21d38049ad0d524c578eed3c5fbedbb"; logging-data="29249"; mail-complaints-to="abuse@eternal-september.org"; posting-account="U2FsdGVkX19EzrHBgNqztzEW6LC9dzXQ" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) Cancel-Lock: sha1:pk0SahvEcyWuxFna9gMMtUapBK0= sha1:j81g8oXXbfASpYjQXBh6xMAV9o8= Xref: csiph.com comp.lang.c:98025 Alla _ writes: > Please, take a look at my reading of the program (in comments to each > statement) that uses pointers to two-dimensional arrays. Is my > understanding correct, i.e. do correctly interpret the given code? > And, please, help me to understand one part closer to the end of the > program. I presume this is code you got from someone else, not something you wrote. The indentation is inconsistent and IMHO excessive. The comments are far more verbose than you'd want in real-world code, but the program is obviously intended for teaching, not production. There is no pointer to a two-dimensional array anywhere in the code. Such a pointer might be defined like this: int (*p)[10][20] The program creates a data structure that acts like a two-dimensional array, but isn't one (and the way the program does it is likely to be more useful and flexible than an actual 2D array). It creates an array of int* pointers, each of which points to an array of ints, with everything allocated using malloc(), with an int** pointing to the initial element of the array of pointers. It's similar to the layout used by argv. > #include > #include > int main(void) > { > // define the number of rows and columns > int nrows = 5; > int ncols = 10; > int row; > > // initialise a pointer, which points to a pointer, which, in turn, points to type int, > // i.e. stores the address of an integer value > int **rowptr; The comment is incorrect. This declares and defines a pointer object, but it doesn't *initialize* anything. Also, "the address of an integer value" doesn't make sense; values don't have addresses. A pointer points to an object, not to a value. > /*** define rowptr pointer by assigning to it the value of the address of the array, > allocated by the call to malloc for nrows (5 in this case) pointers to type int; > therefore the value of the rowptr after the call to malloc will be the address, > at which the first element in the array of pointers to integer type could be stored; > of course, none of these pointers yet point anywhere > ***/ rowptr holds the address of *the initial element of* the array, not the address of the array itself. The two refer to the same location in memory, but with different types. It's common to refer to a pointer to (equivalently, the address of) the initial element of an array as a pointer to the array itself, but it's incorrect. > rowptr = malloc(nrows * sizeof(int *)); This is ok, but it would be better written using the usual idiom: rowptr = malloc(nrows * sizeof *rowptr); You can add parentheses around *rowptr if you think that makes it clearer (personally, I don't). > /** check of the memory has been allocated; if not, print the info message, > and exit **/ > if (rowptr == NULL) > { > puts("\nFailure to allocate room for row pointers.\n"); > exit(0); > } Error messages should be printed to stderr, and program failure should be indicated by returning a non-zero status, preferably EXIT_FAILURE. if (rowptr == NULL) { fputs("Failure to allocate room for row pointers.\n", stderr); exit(EXIT_FAILURE); } Exiting on a malloc failure is a bit drastic, but it's ok for a small demo program like this. > // Print the title > printf("\n\n\nIndex Pointer(hex) Pointer(dec) Diff.(dec)"); The author of this program certainly likes newlines, but omits them at the ends of lines. I see this kind of thing in program written for Windows. Perhaps there's some reason to print three blank lines before the title. Normally every line of output should end in a newline. > /** Allocate memory for an array of ncols integers for every row, and store the value of the > allocated address of the first element of each array at the pointer rowptr[row], > such that rowptr[row] now points to corresponding address of the first element > in the array of ncols integers **/ That description seems to be accurate, apart from a bit of confusion about a pointer "pointing to an address" vs. "*containing* an address" and "pointing to an object". > for (row = 0; row < nrows; row++) > { > rowptr[row] = malloc(ncols * sizeof(int)); Again, I'd write this as: rowptr[row] = malloc(ncols * sizeof *rowptr[row]); > /** check of the memory has been allocated; if not, print the info message, > and exit **/ > if (rowptr[row] == NULL) > { > printf("\nFailure to allocate for row[%d]\n",row); > exit(0); > } And again, the error message should be printed to stderr, and exit(0) should be exit(EXIT_FAILURE). > /** Here is the difficult part: print the number of the current row "row", > print the hexadecimal address of the pointer that now has the value > of the address of the first element of the array of integers; > and the third element should the same address of the pointer > but its decimal value; > > When I tried to compile this, I have received an error message, > which I solved by casting (void *) to the first call to rowptr[row] > (see my new printf below the original one), which is specified > by %p; but I can't solve the second error message (see below) > and I don't understand why the author has not mentioned anything > about it: > error: format specifies type 'int' but the argument has > type 'int *' [-Werror,-Wformat] > printf("\n%d %p %d", row, rowptr[row], rowptr[row]); > ~~ ^~~~~~~~~~~ > > **/ > > /** original printf function: > printf("\n%d %p %d", row, rowptr[row], rowptr[row]); **/ This is attempting to print pointer values as integers. The original author seems to be unaware that this has undefined behavior, and particularly on a system where int and pointers have different sizes it can fail badly. row is of type int, so of course %d is the correct format. The code uses %p to print the value of rowptr[row], which is of type int*. That's almost correct, but %p requires an argument of type void*, not int*. The two types have the same representation on *most* systems, so that's likely to "work", but there are no guarantees. You need to cast the value to void* (as you've done below). Note that the author assumes that %p prints pointer values in hexadecimal; that's common, but not specified by the standard. Using "%d" to print the value of rowptr[row] again is likely to "work" on *some* systems, but can fail badly. (On my system, where int is 32 bits and pointers are 64 bits, it appears to discard the high-order bits of the pointer value.) It can be corrected; see below. > // partially corrected printf > printf("\n%d %p %d", row, (void *)rowptr[row], rowptr[row]); You've correctly fixed the first attempt to print the value of rowptr[row]. The final %d is intended to print the pointer value in decimal -- something that might not even make sense. I'd convert the pointer value to uintptr_t and print it: printf("%d %p %llu\n", row, (void*)rowptr[row], (unsigned long long)(uintptr_t)rowptr[row]); Note that this requires adding "#include " at the top. I convert the pointer value to uintptr_t because that conversion is well defined *if* the implementation defines uintptr_t. I then convert it to unsigned long long so I can print it with "%llu". I could *probably* get away with converting directly to unsigned long long, but I'm being paranoid. (If I were being *really* paranoid, I'd cast rowptr[row] to void* before casting it to uintptr_t. I might also use uintmax_t rather than unsigned long long, just in case uintptr_t is an extended integer type wider than unsigned long long -- which is very unlikely.) I could have used the PRIuPTR macro defined in to print the uintptr_t value directly, but I can never remember how all those macros are spelled, so I just convert to a known type instead. There's no guarantee that the result of the conversion is meaningful, but on most systems it will be. (Personally I'd just stick with "%p" and count on the system to show me something meaningful.) > /** print the difference between the value of addresses of each row **/ > > if (row > 0) > printf(" %d",(int)(rowptr[row] - rowptr[row-1])); Now this is a bit messed up. The author gets points for using pointer arithmetic and converting the result to int, but the result is not particularly meaningful. The pointers being subtracted are of type int*, so the difference is in units of sizeof(int) not in bytes. The decimal values are likely to be byte addresses, so the displayed differences aren't going to correspond to them. That might be interesting, but there are no comments indicating that the author is aware of this important distinction. Furthermore, the two pointers point to distinct objects, allocated by separate calls to malloc(), so subtracting them has undefined behavior. Since malloc() allocates strictly allocated memory, the difference is likely to be a whole number of units of sizeof (int*) bytes, and the output might give some insight into how the system's malloc() behaves, but there's no indication that the author really knows what's going on. Perhaps there's more context at the place where you got this program? On my system, the pointers differ by 48 bytes, which is probably an indication of how much overhead when malloc allocates 8 bytes (sizeof (int*)). > } > return 0; > } Here's my version of the program, without the comments: #include #include #include int main(void) { const int nrows = 5; const int ncols = 10; int **rowptr = malloc(nrows * sizeof *rowptr); if (rowptr == NULL) { fputs("Failure to allocate room for row pointers.\n", stderr); exit(EXIT_FAILURE); } printf("Index Pointer(hex) Pointer(dec) Diff.(dec)\n"); for (int row = 0; row < nrows; row++) { rowptr[row] = malloc(ncols * sizeof *rowptr[row]); if (rowptr[row] == NULL) { fprintf(stderr, "Failure to allocate for row %d\n", row); exit(EXIT_FAILURE); } printf("%d %p %llu", row, (void *) rowptr[row], (unsigned long long)(uintptr_t)rowptr[row]); if (row > 0) { printf(" %d", (int)((char*)rowptr[row] - (char*)rowptr[row-1])); } putchar('\n'); } return 0; } The output on my system is: Index Pointer(hex) Pointer(dec) Diff.(dec) 0 0x55c044d51450 94284276896848 1 0x55c044d51480 94284276896896 48 2 0x55c044d514b0 94284276896944 48 3 0x55c044d514e0 94284276896992 48 4 0x55c044d51510 94284276897040 48 The output when I compile in 32-bit mode (gcc -m32) is: Index Pointer(hex) Pointer(dec) Diff.(dec) 0 0x57726428 1467114536 1 0x57726458 1467114584 48 2 0x57726488 1467114632 48 3 0x577264b8 1467114680 48 4 0x577264e8 1467114728 48 (Aligning the columns would be an interesting exercise.) -- Keith Thompson (The_Other_Keith) kst-u@mib.org Working, but not speaking, for JetHead Development, Inc. "We must do something. This is something. Therefore, we must do this." -- Antony Jay and Jonathan Lynn, "Yes Minister"