Hi Maxime,
On Wed, Feb 24, 2016 at 10:35 AM, Chevallier Maxime
<maxime.chevallier@xxxxxxxxxxx> wrote:
> write-value, write-long-value and write-prepare were parsing
> bytes using strtol with base '0' and restraining wtring size to
> be exactly 2, forbidding to write values over 99. The string length
> is no more checked, we instead check that the parsed value is in the
> correct range.
There seems to be a space in the subject in between ':' please remove that.
> ---
> tools/btgatt-client.c | 41 +++++++++++++----------------------------
> 1 file changed, 13 insertions(+), 28 deletions(-)
>
> diff --git a/tools/btgatt-client.c b/tools/btgatt-client.c
> index 0f6a1bd..3a0ec62 100644
> --- a/tools/btgatt-client.c
> +++ b/tools/btgatt-client.c
> @@ -658,7 +658,7 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
>
> static void cmd_write_value(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 1;
> @@ -726,19 +726,14 @@ static void cmd_write_value(struct client *cli, char *cmd_str)
> }
>
> for (i = 1; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n",
> - argv[i]);
> - goto done;
> - }
> -
> - value[i-1] = strtol(argv[i], &endptr, 0);
> + val = strtol(argv[i], &endptr, 0);
> if (endptr == argv[i] || *endptr != '\0'
> - || errno == ERANGE) {
> + || errno == ERANGE || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n",
> argv[i]);
> goto done;
> }
> + value[i-1] = val;
> }
> }
>
> @@ -793,7 +788,7 @@ static void write_long_cb(bool success, bool reliable_error, uint8_t att_ecode,
>
> static void cmd_write_long_value(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 1;
> @@ -865,21 +860,15 @@ static void cmd_write_long_value(struct client *cli, char *cmd_str)
> }
>
> for (i = 2; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n",
> - argv[i]);
> - free(value);
> - return;
> - }
> -
> - value[i-2] = strtol(argv[i], &endptr, 0);
> + val = strtol(argv[i], &endptr, 0);
> if (endptr == argv[i] || *endptr != '\0'
> - || errno == ERANGE) {
> + || errno == ERANGE || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n",
> argv[i]);
> free(value);
> return;
> }
> + value[i-2] = val;
> }
> }
>
> @@ -909,7 +898,7 @@ static struct option write_prepare_options[] = {
>
> static void cmd_write_prepare(struct client *cli, char *cmd_str)
> {
> - int opt, i;
> + int opt, i, val;
> char *argvbuf[516];
> char **argv = argvbuf;
> int argc = 0;
> @@ -1002,18 +991,14 @@ static void cmd_write_prepare(struct client *cli, char *cmd_str)
> }
>
> for (i = 2; i < argc; i++) {
> - if (strlen(argv[i]) != 2) {
> - printf("Invalid value byte: %s\n", argv[i]);
> - free(value);
> - return;
> - }
> -
> - value[i-2] = strtol(argv[i], &endptr, 0);
> - if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE) {
> + val = strtol(argv[i], &endptr, 0);
> + if (endptr == argv[i] || *endptr != '\0' || errno == ERANGE
> + || val < 0 || val > 255) {
> printf("Invalid value byte: %s\n", argv[i]);
> free(value);
> return;
> }
> + value[i-2] = val;
> }
>
> done:
> --
> 2.1.4
Overall the patch looks pretty good but please fix the following:
WARNING:LONG_LINE: line over 80 characters
#28: FILE: tools/btgatt-client.c:731:
+ || errno == ERANGE || val < 0 || val > 255) {
WARNING:LONG_LINE: line over 80 characters
#61: FILE: tools/btgatt-client.c:865:
+ || errno == ERANGE || val < 0 || val > 255) {
WARNING:LONG_LINE: line over 80 characters
#94: FILE: tools/btgatt-client.c:996:
+ || val < 0 || val > 255) {
--
Luiz Augusto von Dentz
--
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
