On gio, mag 05, 2011 at 03:34:24 +0200, Andrew Lunn wrote:
- uevent_env[0] = kmalloc(strlen("BATTYPE=") +
strlen(uev_type_str[type]) + 1,GFP_ATOMIC);- if (!uevent_env[0])
goto out;- sprintf(uevent_env[0], "BATTYPE=%s", uev_type_str[type]);
Hi Antonio
Hi Andrew,
I don't particularly like having BATTYPE= twice, once in the kmalloc and a second time in the sprintf. Maybe somebody will decide that BATUTYPE is a better name, change the sprintf, forget about the kmalloc, and overflow the allocated memory by one byte. snprintf will prevent the corruption. I've made this sort of stupid error myself, and it takes longer to debug than to write a bit more defensive code which prevents the error.
I definitely agree. I thin that using a define like #define UEV_TYPE_VAR "BATTYPE=" would be more elegant. In this case I can avoid to use snprintf. Do you agree on this?
- /* If the event is DEL, ignore the data field */
- if (action == UEV_DEL)
goto throw;I would replace this goto with a plain if statement.
Mh..ok. I abused of this if->goto style even if not really needed
- if (ret)
bat_dbg(DBG_BATMAN, bat_priv, "Impossible to send ""uevent for (%s,%s,%s) event\n",uev_type_str[type], uev_action_str[action],(action == UEV_DEL ? "NULL" : data));The value of ret could be interesting here, especially if kobject_uevent_env() failed.
Mh, Ok. I can print it into the message. Is there a function in the kernel to transform it in a proper string?
Thank you very much.
Regards,