On 02/19/2013 06:21 PM, Zach Brown wrote:
>> Of course, if after string_list_free() some dynamically allocated
>> strings are used then bad things could happen.
> 
> Right.  So let's not make that even possible by not having the code at
> all.
> 
>> Sorry I don't understand the differences between {leaked, scaled,
>> raw}_string. Could you elaborate a bit ?
> 
> The code I saw returned an allocated string that the caller has to worry
> about.  Crummy code just ignores the problem.  You added the global list
> of strings to free at some point in the future.
> 
> I'm suggesting it not allocate at all so that there's nothing to free.
> 
> Instead of:
> 
>       printf("%s", pretty(value));
> 
>       char *pretty(u64 value) {
>               static char *units[] = { "KB", "MB", /* etc */ };
>               char *str = malloc(20); /*  should be 21 */
>               sprintf(str, "%llu%s",
>                       scale(value), units[scale_index(value));
>               global_list_stuff(str);
>               return str;
>       }
> 
> Do:
> 
>       printf("%llu%s", scale(value), unit_string(value));
> 
>       char *unit(u64 value)
>       {
>               static char *units[] = { "KB", "MB", /* etc */ };
>               return units[scale_index(value));
>       }
> 
> (all rough code, obviously)
> 
> Then there's nothing for the caller to worry about.


Sorry but this is very dangerous and it leads to very subtle bug: what
happens if someone wrote:

printf("%d%s - %d%s\n", scale(123), unit_string(123),
        scale(123), unit_string(456) );



> 
> Right?
> 
> - z
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to