[Freeciv-Dev] Re: (PR#3446) Review results.

["Remi Bonnet" <remi.bonnet@xxxxxxxxxxx>]

> then in gtk-2.0,
>
>+    gtk_list_store_set(scores_store, &it,
>+                       0, (gchar *)get_player(i)->name,
>+                       1, packet->score[i],
>
>and in gtk,
>+  for (i = 0; i < packet->nscores; i++) {
>+    row[0] = get_player(i)->name;
>+    my_snprintf(stat[1], sizeof(stat[1]), "%d", packet->score[i]);
>
>the code assumes that the player id is the rank of the player in the
>packet. But it is wrong! the packet is not sort by id but by score!
>  
>
>
>Fixed.
>
Sorry, i forgot: the same issue happens in others gui

>>in the non-supported gui's:
>>+  char buffer[65536];
>>a bit large...
>>    
>>
>
>With 30 players, this can get very large, reduced it to 32768.
>  
>
In these gui, each player adds a really short string (31 + 
strlen(nation) + strlen(ruler) + strlen(score) ). With
strlen(nation) = 40; strlen(ruler) = 60 and strlen(score) = 19 (these 
values are *really* high), you have only 150 char by player. So i would 
suggest  150 * MAX_NUM_PLAYERS (actually it is 4500 but keep the 
constant so the code will not break when this define grows).

>  
>
>>in the gui-gtk:
>>row is useless: it is created with stat:
>>
>>+  gchar *row[NUM_SCORE_COLS];
>>+  char stat[NUM_SCORE_COLS][64];
>>
>>then points to the same memory:
>>
>>+  for (i = 0; i < ARRAY_SIZE(row); i++) {
>>+    row[i] = stat[i];
>>+  }
>>....
>>So you can replace all row by stat
>>    
>>
>
>This is an array of arrays. It's done just like this many other places
>in repodlgs.c, eg. economy_report_dialog_update().
>
This is not the same. in this function row is used to put the four 
arrays in the same array because gtk_clist_append needs a gchar** in his 
second parameter and the buf? are only four *separate* arrays. But in 
your function, stat is already a char** == gchar**.

Re-tested and no other problems found. I think that after these ones are 
resolved, the patch is ready for CVS.

Remi