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