Fix OOPS in show_uevent()

The last hunk in this patch fixes this.

The other problem is that the envp passed to bus, type and platform callbacks from dev_uevent() is the same, so the callbacks can overwrite the info, written by the others. Did I miss something important?

This is for 2.6.23-rc2-mm1

Signed-off-by Pavel Emelyanov <[email protected]>

---

drivers/base/core.c renamed from drivers/base/core.c.ubug +10 -0

 	}
 
 	if (dev->class && dev->class->dev_uevent) {

@@ -230,6 +235,11 @@ static int dev_uevent(struct kset *kset,
 		if (retval)
 			pr_debug("%s: class uevent() returned %d\n",
 				 __FUNCTION__, retval);
+
+		while (*envp != NULL) {
+			envp++;
+			num_envp--;
+		}
 	}
 
 	if (dev->type && dev->type->uevent) {

@@ -238,6 +248,11 @@ static int dev_uevent(struct kset *kset,
 		if (retval)
 			pr_debug("%s: dev_type uevent() returned %d\n",
 				 __FUNCTION__, retval);
+
+		while (*envp != NULL) {
+			envp++;
+			num_envp--;
+		}
 	}
 
 	return retval;

drivers/base/platform.c renamed from drivers/base/platform.c.ubug +1 -0

@@ -547,6 +547,7 @@ static int platform_uevent(struct device
 
 	envp[0] = buffer;
 	snprintf(buffer, buffer_size, "MODALIAS=%s", pdev->name);
+	envp[1] = NULL;
 	return 0;
 }
 

The platform_uevent() callback called via
show_uevent()
dev_uevent()
platform_uevent()
forgot to set NULL to the last envp pointer and this caused the show_uevent() oops while printing all the envp pointers like this:

The last hunk in this patch fixes this.

The other problem is that the envp passed to bus, type and platform callbacks from dev_uevent() is the same, so the callbacks can overwrite the info, written by the others. Did I miss something important?

But we still don't update the remaining buffer size and the remaining array fields which are left after the call. Shouldn't we instead just change the:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp,
char *buffer, int buffer_size);
to:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp, int *cur_index, char *buffer, int buffer_size, int *cur_len);

like we do for:
int add_uevent_var(char **envp, int num_envp, int *cur_index,
char *buffer, int buffer_size, int *cur_len, const char *format, ...)

and along with the change of the callers, we would update the values properly, so the next call has the correct numbers? There are 6 classes and something like 12 buses using this method, so it shouldn't be too much trouble.

Quoted 16 lines · Kay Sievers

But we still don't update the remaining buffer size and the remaining array fields which are left after the call. Shouldn't we instead just change the:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp,
char *buffer, int buffer_size);
to:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp, int *cur_index, char *buffer, int buffer_size, int *cur_len);

like we do for:
int add_uevent_var(char **envp, int num_envp, int *cur_index,
char *buffer, int buffer_size, int *cur_len, const char *format, ...)

Quoted 4 lines

and along with the change of the callers, we would update the values properly, so the next call has the correct numbers? There are 6 classes and something like 12 buses using this method, so it shouldn't be too much trouble.

Sounds like a sensible approach. We may want the remaining non-users to add_uevent_var() at the same time, I guess.

Quoted 19 lines · Cornelia Huck

Quoted 16 lines · Kay Sievers

But we still don't update the remaining buffer size and the remaining array fields which are left after the call. Shouldn't we instead just change the:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp,
char *buffer, int buffer_size);
to:
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp, int *cur_index, char *buffer, int buffer_size, int *cur_len);

like we do for:
int add_uevent_var(char **envp, int num_envp, int *cur_index,
char *buffer, int buffer_size, int *cur_len, const char *format, ...)

Quoted 4 lines

Quoted 4 lines

and along with the change of the callers, we would update the values properly, so the next call has the correct numbers? There are 6 classes and something like 12 buses using this method, so it shouldn't be too much trouble.

isn't it better to change
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp, char *buffer, int buffer_size);
to
int (*dev_uevent)(struct device *dev,
char **envp, int num_envp, char **buffer);
and alter the buffer pointer inside?