Hi Marek,
On Fri, 5 Nov 2021 at 05:24, Marek Behún kabel@kernel.org wrote:
On Thu, 4 Nov 2021 20:02:29 -0600 Simon Glass sjg@chromium.org 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