Re: [PATCH V3 2/3] mtd: add core code reading DT specified part probes

[Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

On Sun, 9 Apr 2017 13:04:06 +0200
Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> static char **mtd_alloc_part_type_table(int nentries)

Oops, s/char **/const char **/

> {
> 	return kzalloc((nentries + 1) * sizeof(*res), GFP_KERNEL);
> }
> 
> static void mtd_free_part_type_table(const char * const *table)
> {
> 	kfree(table);
> }

I realize this might not be suitable for all kind of part-probes
definitions. Some might need to dynamically allocate each string and
expect the core to free them in mtd_free_part_type_table() (the one I
have in mind is the cmdline part-probes parser). Others already have
the strings statically defined or allocated and maintained somewhere
else (this is the case with DT which provides direct access to string
definitions), which means the core shouldn't free them.

I see 3 solutions to this problem:

1/ go back to your initial solution with DT specific functions, and
   wait until someone decides to implement another way to define
   part-probes (cmdline or ACPI) before considering a more complex
   solution
2/ always allocate strings dynamically and let
   mtd_free_part_type_table() free them. This implies using kstrdup() on
   strings returned by of_property_read_string_array()
3/ use something smarter to let the part-probes table creator free it,
   for example, by using something like:

	struct mtd_part_probes {
		const char * const *types;
		void (*free)(const char * const *types);
	}

#3 is overkill IMO. I'm fine with #1 and #2, pick the one you prefer.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html