Skip to main content
4 of 6
added 2 characters in body
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327

Enable more warnings, and use a memory checker:

gcc-12 -std=c17 -g -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wconversion  -Wstrict-prototypes -fanalyzer 281916.c -o 281916
281916.c: In function ‘PrintArray’:
281916.c:43:20: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
   43 |     for (int i=0; i<array->rows; ++i) {
      |                    ^
281916.c:44:24: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
   44 |         for (int j=0; j<array->cols; ++j) {
      |                        ^
281916.c: At top level:
281916.c:52:5: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
   52 | int main(){
      |     ^~~~
281916.c: In function ‘main’:
281916.c:54:12: warning: unused variable ‘cols’ [-Wunused-variable]
   54 |     size_t cols = 4;
      |            ^~~~
281916.c:53:12: warning: unused variable ‘rows’ [-Wunused-variable]
   53 |     size_t rows = 2;
      |            ^~~~
281916.c: In function ‘NewArray’:
281916.c:17:17: warning: dereference of possibly-NULL ‘array’ [CWE-690] [-Wanalyzer-possible-null-dereference]
   17 |     array->rows = rows;
      |     ~~~~~~~~~~~~^~~~~~
  ‘NewArray’: events 1-4
    |
    |   12 |     if (rows <= 0 || cols <= 0){
    |      |        ^
    |      |        |
    |      |        (1) following ‘false’ branch...
    |......
    |   15 |     struct Array *array = (struct Array *)malloc(sizeof(struct Array *));
    |      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                           |
    |      |                                           (2) ...to here
    |      |                                           (3) this call could return NULL
    |   16 | 
    |   17 |     array->rows = rows;
    |      |     ~~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (4) ‘array’ could be NULL: unchecked value from (3)
    |
281916.c: In function ‘PrintArray’:
281916.c:45:13: warning: use of uninitialized value ‘*_5 + _7’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
   45 |             printf("%.2lf ", array->data[i][j]);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘main’: events 1-2
    |
    |   52 | int main(){
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |......
    |   55 |     struct Array *M = NewArray(2, 4);
    |      |                       ~~~~~~~~~~~~~~
    |      |                       |
    |      |                       (2) calling ‘NewArray’ from ‘main’
    |
    +--> ‘NewArray’: events 3-12
           |
           |   11 | struct Array *NewArray(const size_t rows, const size_t cols){
           |      |               ^~~~~~~~
           |      |               |
           |      |               (3) entry to ‘NewArray’
           |   12 |     if (rows <= 0 || cols <= 0){
           |      |        ~       
           |      |        |
           |      |        (4) following ‘false’ branch...
           |......
           |   15 |     struct Array *array = (struct Array *)malloc(sizeof(struct Array *));
           |      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |                                           |
           |      |                                           (5) ...to here
           |......
           |   21 |     for (size_t i = 0; i < rows; ++i) {
           |      |                        ~~~~~~~~
           |      |                          |
           |      |                          (6) following ‘true’ branch (when ‘i < rows’)...
           |      |                          (9) following ‘true’ branch (when ‘i < rows’)...
           |      |                          (11) following ‘false’ branch (when ‘i >= rows’)...
           |   22 |         array->data[i] = (double *) malloc(cols * sizeof(double));
           |      |                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |                                     |
           |      |                                     (7) ...to here
           |      |                                     (8) region created on heap here
           |      |                                     (10) ...to here
           |......
           |   25 |     return array;
           |      |            ~~~~~
           |      |            |
           |      |            (12) ...to here
           |
    <------+
    |
  ‘main’: events 13-14
    |
    |   55 |     struct Array *M = NewArray(2, 4);
    |      |                       ^~~~~~~~~~~~~~
    |      |                       |
    |      |                       (13) returning to ‘main’ from ‘NewArray’
    |   56 |     PrintArray(M);
    |      |     ~~~~~~~~~~~~~      
    |      |     |
    |      |     (14) calling ‘PrintArray’ from ‘main’
    |
    +--> ‘PrintArray’: events 15-20
           |
           |   42 | void PrintArray(const struct Array *array) {
           |      |      ^~~~~~~~~~
           |      |      |
           |      |      (15) entry to ‘PrintArray’
           |   43 |     for (int i=0; i<array->rows; ++i) {
           |      |                   ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (16) following ‘true’ branch...
           |   44 |         for (int j=0; j<array->cols; ++j) {
           |      |                  ~    ~~~~~~~~~~~~~
           |      |                  |     |
           |      |                  |     (18) following ‘true’ branch...
           |      |                  (17) ...to here
           |   45 |             printf("%.2lf ", array->data[i][j]);
           |      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |             |                     |
           |      |             |                     (19) ...to here
           |      |             (20) use of uninitialized value ‘*_5 + _7’ here
           |
281916.c:45:13: warning: use of uninitialized value ‘*_5 + _7’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
   45 |             printf("%.2lf ", array->data[i][j]);
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘main’: events 1-2
    |
    |   52 | int main(){
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |......
    |   55 |     struct Array *M = NewArray(2, 4);
    |      |                       ~~~~~~~~~~~~~~
    |      |                       |
    |      |                       (2) calling ‘NewArray’ from ‘main’
    |
    +--> ‘NewArray’: events 3-12
           |
           |   11 | struct Array *NewArray(const size_t rows, const size_t cols){
           |      |               ^~~~~~~~
           |      |               |
           |      |               (3) entry to ‘NewArray’
           |   12 |     if (rows <= 0 || cols <= 0){
           |      |        ~       
           |      |        |
           |      |        (4) following ‘false’ branch...
           |......
           |   15 |     struct Array *array = (struct Array *)malloc(sizeof(struct Array *));
           |      |                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |                                           |
           |      |                                           (5) ...to here
           |......
           |   21 |     for (size_t i = 0; i < rows; ++i) {
           |      |                        ~~~~~~~~
           |      |                          |
           |      |                          (6) following ‘true’ branch (when ‘i < rows’)...
           |      |                          (9) following ‘true’ branch (when ‘i < rows’)...
           |      |                          (11) following ‘false’ branch (when ‘i >= rows’)...
           |   22 |         array->data[i] = (double *) malloc(cols * sizeof(double));
           |      |                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |                                     |
           |      |                                     (7) ...to here
           |      |                                     (8) region created on heap here
           |      |                                     (10) ...to here
           |......
           |   25 |     return array;
           |      |            ~~~~~
           |      |            |
           |      |            (12) ...to here
           |
    <------+
    |
  ‘main’: events 13-14
    |
    |   55 |     struct Array *M = NewArray(2, 4);
    |      |                       ^~~~~~~~~~~~~~
    |      |                       |
    |      |                       (13) returning to ‘main’ from ‘NewArray’
    |   56 |     PrintArray(M);
    |      |     ~~~~~~~~~~~~~      
    |      |     |
    |      |     (14) calling ‘PrintArray’ from ‘main’
    |
    +--> ‘PrintArray’: events 15-22
           |
           |   42 | void PrintArray(const struct Array *array) {
           |      |      ^~~~~~~~~~
           |      |      |
           |      |      (15) entry to ‘PrintArray’
           |   43 |     for (int i=0; i<array->rows; ++i) {
           |      |                   ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (16) following ‘true’ branch...
           |   44 |         for (int j=0; j<array->cols; ++j) {
           |      |                  ~    ~~~~~~~~~~~~~
           |      |                  |     |
           |      |                  |     (18) following ‘true’ branch...
           |      |                  |     (20) following ‘true’ branch...
           |      |                  (17) ...to here
           |   45 |             printf("%.2lf ", array->data[i][j]);
           |      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |             |                     |
           |      |             |                     (19) ...to here
           |      |             |                     (21) ...to here
           |      |             (22) use of uninitialized value ‘*_5 + _7’ here
           |
make[2]: Leaving directory '/home/tms/stackexchange/review'
valgrind --leak-check=full ./281916 ==4108801== Memcheck, a memory error detector
==4108801== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==4108801== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==4108801== Command: ./281916
==4108801== 
==4108801== Invalid write of size 8
==4108801==    at 0x1091B8: NewArray (281916.c:17)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801==  Address 0x4a5a048 is 0 bytes after a block of size 8 alloc'd
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x1091AB: NewArray (281916.c:15)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801== 
==4108801== Invalid write of size 8
==4108801==    at 0x1091C4: NewArray (281916.c:18)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801==  Address 0x4a5a050 is 8 bytes after a block of size 8 alloc'd
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x1091AB: NewArray (281916.c:15)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801== 
==4108801== Invalid read of size 8
==4108801==    at 0x109365: PrintArray (281916.c:43)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801==  Address 0x4a5a048 is 0 bytes after a block of size 8 alloc'd
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x1091AB: NewArray (281916.c:15)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801== 
==4108801== Invalid read of size 8
==4108801==    at 0x109344: PrintArray (281916.c:44)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801==  Address 0x4a5a050 is 8 bytes after a block of size 8 alloc'd
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x1091AB: NewArray (281916.c:15)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C8FC8: __printf_fp_l (printf_fp.c:396)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C8FE2: __printf_fp_l (printf_fp.c:396)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C4112: __mpn_extract_double (dbl2mpn.c:56)
==4108801==    by 0x48C958E: __printf_fp_l (printf_fp.c:396)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C4117: __mpn_extract_double (dbl2mpn.c:60)
==4108801==    by 0x48C958E: __printf_fp_l (printf_fp.c:396)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C9897: __printf_fp_l (printf_fp.c:978)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C98D5: __printf_fp_l (printf_fp.c:981)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48CAA78: __printf_fp_l (printf_fp.c:991)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C9926: round_away (rounding-mode.h:52)
==4108801==    by 0x48C9926: __printf_fp_l (printf_fp.c:998)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C9B94: __printf_fp_l (printf_fp.c:1166)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C9F7E: __printf_fp_l (printf_fp.c:1228)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48C9F63: __printf_fp_l (printf_fp.c:1230)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48F81D9: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:782)
==4108801==    by 0x48CA047: __printf_fp_l (printf_fp.c:1254)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D39E4: __vfprintf_internal (vfprintf-internal.c:1062)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D39F4: done_add_func (vfprintf-internal.c:127)
==4108801==    by 0x48D39F4: __vfprintf_internal (vfprintf-internal.c:1067)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D39FC: __vfprintf_internal (vfprintf-internal.c:1067)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D2897: done_add_func (vfprintf-internal.c:127)
==4108801==    by 0x48D2897: outstring_func (vfprintf-internal.c:241)
==4108801==    by 0x48D2897: __vfprintf_internal (vfprintf-internal.c:1096)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D28B0: done_add_func (vfprintf-internal.c:127)
==4108801==    by 0x48D28B0: outstring_func (vfprintf-internal.c:241)
==4108801==    by 0x48D28B0: __vfprintf_internal (vfprintf-internal.c:1096)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Conditional jump or move depends on uninitialised value(s)
==4108801==    at 0x48D28B8: __vfprintf_internal (vfprintf-internal.c:1096)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
==4108801== Syscall param write(buf) points to uninitialised byte(s)
==4108801==    at 0x496E190: write (write.c:26)
==4108801==    by 0x48F6E64: _IO_file_write@@GLIBC_2.2.5 (fileops.c:1180)
==4108801==    by 0x48F621F: new_do_write (fileops.c:448)
==4108801==    by 0x48F7E78: _IO_do_write@@GLIBC_2.2.5 (fileops.c:425)
==4108801==    by 0x48F8282: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:783)
==4108801==    by 0x48EF1B0: putchar (putchar.c:28)
==4108801==    by 0x109356: PrintArray (281916.c:47)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801==  Address 0x4a5a1a2 is 2 bytes inside a block of size 1,024 alloc'd
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x48EB76B: _IO_file_doallocate (filedoalloc.c:101)
==4108801==    by 0x48F8F4F: _IO_doallocbuf (genops.c:347)
==4108801==    by 0x48F8F4F: _IO_doallocbuf (genops.c:342)
==4108801==    by 0x48F8317: _IO_file_overflow@@GLIBC_2.2.5 (fileops.c:744)
==4108801==    by 0x48CA047: __printf_fp_l (printf_fp.c:1254)
==4108801==    by 0x48D3EEC: __printf_fp_spec (vfprintf-internal.c:354)
==4108801==    by 0x48D3EEC: __vfprintf_internal (vfprintf-internal.c:1061)
==4108801==    by 0x48C84FA: printf (printf.c:33)
==4108801==    by 0x109335: PrintArray (281916.c:45)
==4108801==    by 0x1093AB: main (281916.c:56)
==4108801== 
0.00 0.00 0.00 0.00 
0.00 0.00 0.00 0.00 
==4108801== 
==4108801== HEAP SUMMARY:
==4108801==     in use at exit: 88 bytes in 4 blocks
==4108801==   total heap usage: 5 allocs, 1 frees, 1,112 bytes allocated
==4108801== 
==4108801== 88 (8 direct, 80 indirect) bytes in 1 blocks are definitely lost in loss record 3 of 3
==4108801==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4108801==    by 0x1091AB: NewArray (281916.c:15)
==4108801==    by 0x10939B: main (281916.c:55)
==4108801== 
==4108801== LEAK SUMMARY:
==4108801==    definitely lost: 8 bytes in 1 blocks
==4108801==    indirectly lost: 80 bytes in 3 blocks
==4108801==      possibly lost: 0 bytes in 0 blocks
==4108801==    still reachable: 0 bytes in 0 blocks
==4108801==         suppressed: 0 bytes in 0 blocks
==4108801== 
==4108801== Use --track-origins=yes to see where uninitialised values come from
==4108801== For lists of detected and suppressed errors, rerun with: -s
==4108801== ERROR SUMMARY: 194 errors from 24 contexts (suppressed: 0 from 0)

You're missing the most basic of checks, using pointers returned from malloc() when they may be null. You need to fix that immediately.

malloc() returns a void*, which is freely convertible to any kind of object pointer without a cast. Adding a cast serves no purpose, except to clutter the code and sometimes to mask a failure to include <stdlib.h>.

It's easier for reviewers if we can see that the size allocated matches the type of pointer without having to go back to the pointer's definition. Consider these:

    struct Array *array = malloc(sizeof *array);
    array->data = malloc(rows * sizeof *array->data);
        array->data[i] = malloc(cols * sizeof *array->data[i]);

See how they are much more obviously correct, without needing access to the rest of the code? (That's probably why you didn't spot the error in array = (struct Array *)malloc(sizeof(struct Array *))).


FreeArray() looks totally broken:

    if (array != NULL) {
        return -1;
    }
    assert (array->data);

The assert() is reached only if array is null, when dereferencing it in array->data is Undefined Behaviour.

The right thing to do is to accept and ignore a null pointer argument, just like free() does.


Fixing the above problems gives:

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>

struct Array {
    double **data;
    size_t rows;
    size_t cols;
};

struct Array *NewArray(const size_t rows, const size_t cols)
{
    struct Array *array = malloc(sizeof *array);
    if (!array) {
        return array;
    }

    array->rows = rows;
    array->cols = cols;

    array->data = malloc(rows * sizeof *array->data);
    if (!array->data) {
        free(array);
        return NULL;
    }

    for (size_t i = 0; i < rows; ++i) {
        array->data[i] = malloc(cols * sizeof *array->data[i]);
        if (!array->data[i]) {
            while (i-->0) {
                free(array->data[i]);
            }
            free(array->data);
            free(array);
            return NULL;
        }
        for (size_t j = 0;  j < cols;  ++j) {
            array->data[i][j] = 0.0;
        }
    }

    return array;
}

void FreeArray(struct Array *array)
{
    if (array) {
        assert (array->data);
        for (size_t i = 0; i < array->rows; ++i) {
            free(array->data[i]);
        }
        free(array->data);
        free(array);
    }
}

void PrintArray(const struct Array *array)
{
    for (size_t i = 0;  i < array->rows;  ++i) {
        for (size_t j = 0;  j < array->cols;  ++j) {
            printf("%.2lf ", array->data[i][j]);
        }
        printf("\n");
    }
    return;
}

#include <errno.h>
#include <string.h>
int main(void)
{
    struct Array *M = NewArray(2, 4);
    if (!M) {
        fputs(strerror(ENOMEM), stderr);
        return EXIT_FAILURE;
    }
    PrintArray(M);
    FreeArray(M);
}

Reconsider the representation. Using a separate allocation for each row can mean that your data are spread all over the address space, which isn't very cache friendly. Consider a single linear array containing all the columns:

struct Array {
    double *data;
    size_t rows;
    size_t cols;
};

static double *array_element(struct Array *a, size_t row, size_t col)
{
   return a->data + row * a->cols + col;
}

This representation also makes it much easier to clean up when one of the allocations fails.

#include <stdio.h>
#include <stdlib.h>

struct Array {
    double *data;
    size_t rows;
    size_t cols;
};

/* private helper */
static double *array_element(const struct Array *a, size_t row, size_t col)
{
   return a->data + row * a->cols + col;
}

/* public interface */
double array_value(const struct Array *a, size_t row, size_t col)
{
    return *array_element(a, row, col);
}

void array_set_value(const struct Array *a, size_t row, size_t col, double v)
{
    *array_element(a, row, col) = v;
}

struct Array *NewArray(const size_t rows, const size_t cols)
{
    struct Array *array = malloc(sizeof *array);
    if (!array) {
        return array;
    }

    array->data = malloc(sizeof *array->data * rows * cols);
    if (!array->data) {
        free(array);
        return NULL;
    }

    array->rows = rows;
    array->cols = cols;
    for (size_t j = 0;  j < rows * cols;  ++j) {
        array->data[j] = 0.0;
    }

    return array;
}

void FreeArray(struct Array *array)
{
    if (array) {
        free(array->data);
        free(array);
    }
}

void PrintArray(const struct Array *array)
{
    for (size_t i = 0;  i < array->rows;  ++i) {
        for (size_t j = 0;  j < array->cols;  ++j) {
            printf("%.2lf ", array_value(array, i, j));
        }
        printf("\n");
    }
}

#include <errno.h>
#include <string.h>
int main(void)
{
    struct Array *M = NewArray(2, 4);
    if (!M) {
        fputs(strerror(ENOMEM), stderr);
        return EXIT_FAILURE;
    }
    PrintArray(M);
    FreeArray(M);
}
Toby Speight
  • 88.4k
  • 14
  • 104
  • 327