On Mon, Mar 25, 2024 at 4:47 AM Richard Guo <guofengli...@gmail.com> wrote: > 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.
Thank you. I'm going to deal with this in the next few hours. ------ Regards, Alexander Korotkov