Hi Marek, On Fri, 5 Nov 2021 at 05:24, Marek Behún <[email protected]> wrote: > > On Thu, 4 Nov 2021 20:02:29 -0600 > Simon Glass <[email protected]> wrote: > > > Better to make iter a struct sysinfo_str_list_iter, I think and > > require the caller to declare it: > > > > sysinfo_str_list_iter iter; > > char str[80]' > > > > p = sysinfo_str_list_first(dev, i, &str, sizeof(str), &iter); > > ... > > > > Do you need the iter? > > > > If you want to support arbitratry length, I suppose that is OK?? But I > > don't like allocating memory unless it is needed. > > Well if I am iterating through default environment variables > overwrites, they can be basically up to ENV_SIZE long. There may be > some long commands stored there.
OK. > > Another solution would be to redesign sysinfo_get_str() and introduce > sysinfo_get_str_list() so that they won't fill a buffer given by user, > but instead have their own buffer in implementation and return const > char * pointer. Yes that was a design idea at the start...but I think at present I like the current API. I just didn't understand your intent properly. My new thoughts: - pass in the iter so malloc() is not needed there (change str to a char *) - return an int from your iterator functions, so you can tell when you run out of memory and need to die - comment struct sysinfo_str_list_iter - use log_debug() instead of printf(), on error Also I wonder about this: - get the caller to provide the str buffer, and maxsize, so malloc() is not needed I can see the advantage of allocating though. I wonder if you might keep track of the current buffer length and do a realloc() if it is too small, each time? Scanning through to find the max length might be slower? Some drivers may read from an I2C device, for example. Regards, Simon
