Re: [PATCH v3] usb/hcd: Send a uevent signaling that the host controller had died

[Alan Stern]

On Thu, 18 Apr 2019, Greg Kroah-Hartman wrote:

> On Wed, Apr 17, 2019 at 04:20:09PM -0700, Guenter Roeck wrote:
> > On Wed, Apr 17, 2019 at 3:41 PM Raul Rangel <rrangel@xxxxxxxxxxxx> wrote:
> > >
> > > On Wed, Apr 17, 2019 at 03:23:52PM -0700, Guenter Roeck wrote:
> > > > On Wed, Apr 17, 2019 at 3:11 PM Raul Rangel <rrangel@xxxxxxxxxxxx> wrote:
> > > > >
> > > > > On Wed, Apr 17, 2019 at 04:39:23PM -0400, Alan Stern wrote:
> > > > > >
> > > > > > This sounds like a golden opportunity!  Submit a separate patch making
> > > > > > the parameter to kobject_uevent_env be const (actually const char *
> > > > > > const []), then submit this patch on top of that one.
> > > > > So there are other parts of the code base that dynamically create their
> > > > > array values. So by making the function take const, it breaks :(
> > > >
> > > > Confused. The calling code can still be non-const. I don't see the
> > > > parameter modified in kobject_uevent_env(), so declaring it const
> > > > should be possible. Can you give an example of code that no longer
> > > > works ?
> > > static int notify_user_space(struct thermal_zone_device *tz, int trip)
> > > {
> > >         char *thermal_prop[5];
> > >         int i;
> > >
> > >         mutex_lock(&tz->lock);
> > >         thermal_prop[0] = kasprintf(GFP_KERNEL, "NAME=%s", tz->type);
> > >         thermal_prop[1] = kasprintf(GFP_KERNEL, "TEMP=%d", tz->temperature);
> > >         thermal_prop[2] = kasprintf(GFP_KERNEL, "TRIP=%d", trip);
> > >         thermal_prop[3] = kasprintf(GFP_KERNEL, "EVENT=%d", tz->notify_event);
> > >         thermal_prop[4] = NULL;
> > >         kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop);
> > >         for (i = 0; i < 4; ++i)
> > >                 kfree(thermal_prop[i]);
> > >         mutex_unlock(&tz->lock);
> > >         return 0;
> > > }
> > >
> > > drivers/thermal/user_space.c:48:52: error: passing 'char *[5]' to parameter of type 'const char *const *' discards qualifiers in nested pointer types [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
> > >         kobject_uevent_env(&tz->device.kobj, KOBJ_CHANGE, thermal_prop);
> > >                                                           ^~~~~~~~~~~~
> > > include/linux/kobject.h:238:22: note: passing argument to parameter 'envp' here
> > >                         const char *const envp[]);
> > >                                           ^
> > >
> > > http://c-faq.com/ansi/constmismatch.html explains why it fails.
> > >
> > Interesting. One never stops learning. So the best you could do would
> > be char * const envp[], but I guess that doesn't help much.
> 
> Yeah, I went down this path a year or so ago and had to give it up as
> well :(

Well, the signature could still be changed as Guenter suggests.

And the array being added in the new code could still be static.  
After all, there isn't really any danger that the contents of those
strings will be modified, right?  It's just that the const modifiers
weren't put in until it was too late and there were too many existing
callers.  Perhaps a comment about this could be included in the 
kerneldoc for kobject_uevent_env.

Alan Stern