Re: request_firmware() hotplug interface, third round and a halve

Manuel Estrada Sainz (ranty@debian.org)
Sun, 18 May 2003 20:17:31 +0200


On Sun, May 18, 2003 at 08:14:06PM +0400, Alexey Mahotkin wrote:
>
> Two comments on firmware_class_hotplug(), maybe both are irrelevant.
>
> +int firmware_class_hotplug(struct class_device *class_dev, char **envp,
> + int num_envp, char *buffer, int buffer_size)
> +{
> + struct firmware_priv *fw_priv = class_get_devdata(class_dev);
> + int i=0;
> + char *scratch=buffer;
> +
> + if (buffer_size < (FIRMWARE_NAME_MAX+10))
> + return -ENOMEM;
> +
> + envp [i++] = scratch;
> + scratch += sprintf(scratch, "FIRMWARE=%s", fw_priv->fw_id) + 1;
> + return 0;
> +}
> +
>
> First, I do not understand how the environment is handled here.

You can take a look at lib/kobject.c:kset_hotplug()

> You're just setting first element of provided environment to
> "FIRMWARE=%s", possibly overwriting the existing value.

envp points to the first empty slot of the environment being
constructed, so no, I am not overwriting anything.

> Then why are you incrementing `i'? Why are you using `i' at all? Why
> are you incrementing `scratch'?

The code is ready to add a new environment variable if needed. If we
really don't need more variables, it could be simplified, but I don't
think it is so important.

> Ah, it seems like you should be using num_envp somehow, and you're not.

OK, I just added:
if (num_envp < 1)
return -ENOMEM;

> Also, environment pointer list must be terminated with a NULL pointer. Is
> it not done or is that handled somewhere else?

The envp array we get is reset to zero, so anything we don't explicitly
set is already NULL.

> The machine I have 2.5.69 sources is not reachable now so I cannot
> check it. Sorry if I am wrong.

The num_envp issue was real, and anyway, I appreciate a little
feedback, it seams like people don't hack the kernel on Sunday :-P

Thanks

Manuel

-- 
--- Manuel Estrada Sainz <ranty@debian.org>
                         <ranty@bigfoot.com>
			 <ranty@users.sourceforge.net>
------------------------ <manuel.estrada@hispalinux.es> -------------------
Let us have the serenity to accept the things we cannot change, courage to
change the things we can, and wisdom to know the difference.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/