[Freeciv-Dev] Re: (PR#8504) Add function to get all section entries

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8504 >

Raimar Falke wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8504 >
> 
> 
> For things like
> 
> [key_bindings]
> c="center_unit"
> C="center_capital"
> 
> Kp8="move_unit(n)"
> Kp6="move_unit(e)"
> Kp2="move_unit(s)"
> Kp4="move_unit(w)"
> 
> Kp7="move_unit(nw)"
> Kp9="move_unit(ne)"
> Kp3="move_unit(se)"
> Kp1="move_unit(sw)"
> 
> Left="scroll(left,1,tile)"
> Right="scroll(right,1,tile)"
> Up="scroll(up,1,tile)"
> Down="scroll(down,1,tile)"
> 
> Shift-Left="scroll(left,0.5,screen)"
> Shift-Right="scroll(right,0.5,screen)"
> Shift-Up="scroll(up,0.5,screen)"
> Shift-Down="scroll(down,0.5,screen)"
> 
> Alt-Left="scroll(left,1,pixel)"
> Alt-Right="scroll(right,1,pixel)"
> Alt-Up="scroll(up,1,pixel)"
> Alt-Down="scroll(down,1,pixel)"
> 
> I need a way to iterate over all keys of the given section. The
> attached patch add such a function.

I'm pretty sure you don't *need* this.  Instead of doing

   entries = secfile_get_section_entries(file, "key_bindings", &n);

   for (i = 0; i < n; i++) {
     interpret_key_binding(entries[i]);
   }
   free(entries);

You could do

   for (key = 0; key < NUM_KEYS; i++) {
     char *keyname = keys[key];
     char *keyval = secfile_lookup_string_default(file, NULL, ...);

     if (keyval) {
       interpret_key_binding(keyval);
     }
   }

this might be more reliable, but may also be more work (you need a list 
of keys - but you probably need this list anyway...).

Anyway, the function sounds like it is generally useful.

> +  section_list_iterate_rev(*my_section_file->sections, asection) {
> +    if (strcmp(asection->name, section) == 0) {
> +      psection = asection;
> +      break;
> +    }
> +  } section_list_iterate_rev_end;

This should become a helper function.  There's code just like this in 
several places (lines 418, 1146, 1202).

Why do you use section_list_iterate_rev instead of section_list_iterate 
like everyone else?  The only other place section_list_iterate_rev is 
used is for adding to a secfile, on the assumption that the last-entered 
section is most likely to be added to again.  This doesn't apply when 
reading a file.

> +      *num=0;

Bad style.

> +  ret = fc_malloc((*num) * sizeof(char *));

Should be sizeof(*ret)

jason