Re: [AD] mode selection

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> updated, my porting to 4.1.10 introduced a bug.

+   if (is_autodetect ||
+    !(gfx_mode_list = get_gfx_mode_list(driver_list_entry->id))) {
+      size_t n, max_count = sizeof(default_mode_list) / sizeof(MODE_LIST);
       driver_list_entry->mode_count = 0;
+      driver_list_entry->mode_list = NULL;
+      driver_list_entry->fetch_mode_list_ptr = NULL;

You leak memory by setting driver_list_entry->fetch_mode_list_ptr to NULL 
while dynamically allocating the list. And I think that using dynamic 
allocation even though there is no sieve function is inefficient.

The code that dynamically grows the list is basically written twice.


-int gfx_mode_select_ex(int *card, int *w, int *h, int *color_depth)
+int gfx_mode_select_sieve(
+   int *card, int *w, int *h, int *color_depth,
+   int (*sieve)(int,int,int,int)
+)

Ouch! Weird formatting...


+int gfx_mode_select_ex(int *card, int *w, int *h, int *color_depth)
+{
+   ASSERT(card);
+   ASSERT(w);
+   ASSERT(h);
+   ASSERT(color_depth);
+   return gfx_mode_select_sieve(card, w, h, color_depth, NULL);

Take a look at what the function just below passes as 'color_depth'. You need 
either to remove the 4th assert or redirect gfx_mode_select().


+@@int @gfx_mode_select_sieve(int *card, int *w, int *h, int ();
+@xref gfx_mode_select, gfx_mode_select_ex, set_color_depth, set_gfx_mode, 
gui_fg_color
+   Displays the Allegro graphics mode selection dialog, which allows the 
+   user to select a screen mode and graphics card. Stores the selection in 
+   the three variables, and returns zero if it was closed with the Cancel 
+   button or non-zero if it was OK'd.

Hum...


I renamed gfx_mode_select_sieve into gfx_mode_select_filter and commited the 
attached patch to mainline.

-- 
Eric Botcazou

Attachment: modesel_filter.diff.gz
Description: GNU Zip compressed data