Hi Antonio,
On Tue, May 06, 2014, Antonio Ospite wrote:
> static gboolean setup_leds(GIOChannel *channel, GIOCondition cond,
> gpointer user_data)
> {
> - int number = GPOINTER_TO_INT(user_data);
> - uint8_t bitmap = 0;
> int fd;
> + int i;
> + int ret;
> + struct leds_data *data = (struct leds_data *)user_data;
No need to have an explicit cast for a void pointer.
>
> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
> + if (data == NULL)
> return FALSE;
>
> - DBG("number %d", number);
> + if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
> + goto out;
>
> fd = g_io_channel_unix_get_fd(channel);
>
> - if (calc_leds_bitmap(number, &bitmap))
> - set_leds_hidraw(fd, bitmap);
> + set_leds_hidraw(fd, data->bitmap);
> +
> +out:
> + for (i = 0; i < 4; i++)
> + free(data->syspaths[i]);
> + free(data);
>
> return FALSE;
> }
I don't see you using the "int ret" anywhere in this function. Even if
you do end up using it later for returning something I suppose it should
be a boolean type and not int. However even then the variable should be
introduced in the patch that actually makes use of it. If you build test
each patch individually with --enable-maintainer-mode the compiler
should warn about unused variables (amongst many other things) for you.
> @@ -313,6 +324,35 @@ static int get_js_number(struct udev_device *udevice)
> return number;
> }
>
> +static struct leds_data *get_leds_data(struct udev_device *udevice)
> +{
> + struct leds_data *data;
> + int number;
> + int i;
> + int ret;
> +
> + data = malloc(sizeof(*data));
> + if (data == NULL)
> + return NULL;
> +
> + memset(data, 0, sizeof(*data));
> +
> + number = get_js_number(udevice);
> + DBG("number %d", number);
> +
> + ret = calc_leds_bitmap(number, &data->bitmap);
> + if (ret == FALSE)
> + goto err;
Same here: if you're storing a boolean value into ret it should be
declared as a boolean type instead of int. Also, you don't need the
"== FALSE" comparison since boolean types are supposed to be valid
boolean expressions by themselves, i.e. "if (!ret)" should do.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
