Re: Bug with lists in tables in ui-out.c

[Andrew Cagney <ac131313@cygnus.com>]

> Hi all,

This code my head hurt.

> Turns out if you have an element of a table that is a list or tuple, then
> the current ui-out table code chokes.  verify_field_alignment doesn't know
> that each of the elements of the sublist are not separate table elements, so
> it throws an error at the first one it sees.  The following patch fixes this
> bug.  

Can this be fixed in a simpler cleaner way by just moving the table 
stuff into `struct ui_out_level'?

When I was last working on this code I very nearly did just this change :-/

enjoy,
Andrew

> Note that in the FSF gdb code, no one ever uses this feature, because the
> only client of the ui_out_table is the breakpoint printer, and it cleverly
> only declares the first 5 or 6 columns of the breakpoint info table in the
> table header, and sticks on the rest of the info as undeclared freebies
> (which is kind of lame, but another story...), so the commands, which are a
> list, don't get added to the table proper, and thus don't trip the bug.
> 
> I have a patch which will fix this and make the table header accurate, but I
> am not submitting it here because (a) it is a separate issue and (b) it will
> change the output of the "info breakpoint" command (though only the header),
> and so may not necessarily please everyone...
> 
> Jim
> 
> Index: ui-out.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ui-out.c,v
> retrieving revision 1.18
> diff -p -r1.18 ui-out.c
> *** ui-out.c    2001/07/06 03:53:11     1.18
> --- ui-out.c    2001/11/29 01:40:59
> *************** ui_out_end (struct ui_out *uiout,
> *** 367,372 ****
> --- 367,384 ----
>             enum ui_out_type type)
>   {
>     int old_level = pop_level (uiout, type);
> + 
> +   /* If we are building up a table, and we were making a table element
> +      which was a list or tuple, then when we end the list, we have to
> +      increment the header. Watch out, however, the current breakpoint
> +      code actually emits more columns than it defines in the header
> +      so we have to be careful not to walk off the end. */
> + 
> +   if (uiout->table_flag && uiout->body_flag
> +       && uiout->level == 1
> +       && uiout->headercurr != NULL)
> +     uiout->headercurr = uiout->headercurr->next;
> +   
>     uo_end (uiout, type, old_level);
>   }
>   
> *************** append_header_to_list (struct ui_out *ui
> *** 1018,1024 ****
>     uiout->headercurr = uiout->headerlast;
>   }
>   
> ! /* returns 0 if there is no more headers */
>   
>   static int
>   get_curr_header (struct ui_out *uiout,
> --- 1030,1038 ----
>     uiout->headercurr = uiout->headerlast;
>   }
>   
> ! /* Returns information on the current header.  ALSO Increments the
> !    current header in UI_OUT.  Returns 0 if there is no more headers, 1
> !    otherwise. */
>   
>   static int
>   get_curr_header (struct ui_out *uiout,
> *************** specified after table_body and inside a
> *** 1056,1062 ****
>       }
>   }
>   
> ! /* determines what is the alignment policy */
>   
>   static void
>   verify_field_alignment (struct ui_out *uiout,
> --- 1070,1079 ----
>       }
>   }
>   
> ! /* Determines what is the alignment policy.  NB: as a side-effect of
> !  calling get_curr_header, this also increments the current header.  So
> !  you have to call verify_field_alignment in any call that adds a
> !  complete bit of data to a table column. */
>   
>   static void
>   verify_field_alignment (struct ui_out *uiout,
> *************** verify_field_alignment (struct ui_out *u
> *** 1066,1073 ****
>   {
>     int colno;
>     char *text;
>   
> !   if (uiout->table_flag
>         && get_curr_header (uiout, &colno, width, align, &text))
>       {
>         if (fldno != colno)
> --- 1083,1097 ----
>   {
>     int colno;
>     char *text;
> + 
> +   /* Be careful - if you are building up a list or tuple AS AN ELEMENT
> +      of a table, you don't want to call get_curr_header, since that will
> +      increment the current header, which would be wrong. Currently, tables
> +      can only be built at level 1, so I am using that as a check.  If this
> +      stuff is ever generalized, we will need to stuff the level of the
> +      current table into the (putative) ui_table structure. */
>   
> !   if (uiout->table_flag && uiout->level == 1
>         && get_curr_header (uiout, &colno, width, align, &text))
>       {
>         if (fldno != colno)
> 
>