[Freeciv-Dev] Re: Split patch (was Re: [RFC PATCH] init_techs)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

I like your statement of what you claim to be agreeing to followed
by what you actually want to do in practice.

Why on earth would you handle the same buffer contents 3 times with
all those allocation and free calls when you could do it once and
let the caller deal with at most one extra copy depending on whether
the original buffer or the results needed to be preserved.

But most of the time I would expect that no extra copies were required
in the caller algorithms.

split should treat the buffer it was handed as working memory, return
pointers into the parsed string elements, and let the caller deal with
ALL memory issues.

It is really the only sensible general purpose solution for something 
like this.

Cheers,
RossW
=====

At 11:31 AM 01/09/27 +0200, Raimar Falke wrote:
>On Wed, Sep 26, 2001 at 07:44:25PM -0400, Justin Moore wrote:
[...]
>>    I think I need some sample code to see how this works.  Even psuedocode
>> could go a long ways.  Maybe I'm just missing something simple here.
>> Given this function declaration:
>> 
>> int split(const char *toks, char *buf, char *args[], const int maxargs);
>> 
>> what happens where, allocation-wise?
>
>Ross agrees with me (mark this day in the calendar) that the caller
>should do the allocation. So the function declaration would be:
>
>int split(const char *const toks, const char *const string, 
>          char *items[], int max_items, int max_item_len)

int split(const char *const toks, char *buffer, char *items[], int max_items)

This is about what the interface should be.

>{
>   char *mycopy=strdup(string);
Unnecessary garbage, but at least it is cleaned up before returning.

>   int i=0;
>
>   for(i=0;i<max_items;i++)
You certainly don't iterate over max_items, just check to make sure
the next string pointer doesn't exceed max_items :-)

>   {
>      char *token;
>      ....
>      mystrlcpy(items[i],token,max_item_len);
More unnecessary garbage, this one usually generates tons of memory leaks
from absent minded programmers, or those that can't tell 5 levels up
who allocated or should deallocate this. Especially bad for 1st or 2nd
generation maintainers who never really wrote or dealt with this in the
first place. VERY bad programming practice to put such things in utility
routines.

>   }
>   free(mycopy);
>}
>
>       Raimar
>
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
>  "brand memory are for windows users that think their stability
>   problems come from the memory"
>    -- bomek in #freeciv

Cheers,
RossW
=====