On Mon, Mar 25, 2024 at 10:07 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Alexander Korotkov <aekorot...@gmail.com> writes: > > Here goes the revised patch. I'm going to push this if there are no > objections. > > Quite a lot of the buildfarm is complaining about this: > > reindexdb.c: In function 'reindex_one_database': > reindexdb.c:434:54: error: 'indices_tables_cell' may be used uninitialized > in this function [-Werror=maybe-uninitialized] > 434 | strcmp(prev_index_table_name, indices_tables_cell->val) == 0) > | ~~~~~~~~~~~~~~~~~~~^~~~~ > > I noticed it first on mamba, which is set up with -Werror, but a > scrape of the buildfarm logs shows many other animals reporting this > as a warning. I noticed the similar warning on cfbot: https://cirrus-ci.com/task/6298504306360320?logs=gcc_warning#L448 reindexdb.c: In function ‘reindex_one_database’: reindexdb.c:437:24: error: ‘indices_tables_cell’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 437 | indices_tables_cell = indices_tables_cell->next; | ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ Although it's complaining on line 437 not 434, I think they are the same issue. > I think they are right. Even granting that the > compiler realizes that "parallel && process_type == REINDEX_INDEX" is > enough to reach the one place where indices_tables_cell is > initialized, that's not really enough, because that place is > > if (indices_tables_list) > indices_tables_cell = indices_tables_list->head; > > So I believe this code will crash if get_parallel_object_list returns > an empty list. Initializing indices_tables_cell to NULL in its > declaration would stop the compiler warning, but if I'm right it > will do nothing to prevent that crash. This needs a bit more effort. Agreed. And the comment of get_parallel_object_list() says that it may indeed return NULL. BTW, on line 373, it checks 'process_list' and bails out if this list is NULL. But it seems to me that 'process_list' cannot be NULL in this case, because it's initialized to be 'user_list' and we have asserted that user_list is not NULL on line 360. I wonder if we should check indices_tables_list instead of process_list on line 373. Thanks Richard
