greybus-dev

greybus-dev@lists.linaro.org

10 participants
671 discussions

[PATCH v5] staging: greybus: loop_backtest: fixed consistent spacing style issue

by Jonathan Bowie

Fixed incosistent spacing around arithmetic operator. Signed-off-by: Jonathan Bowie <eudjtb(a)gmail.com> --- v2 -fixed subject added changelog v3 -CC:ed linux-next incorrectly, added proper summary v4 -CC:ed correct mailing list, moved changelog to summary body v5 -correctly placed changelog drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..d5bb8d2 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -476,7 +476,7 @@ int format_output(struct loopback_test *t, r->gbphy_firmware_latency_jitter); } else { - len += snprintf(&buf[len], buf_len- len, ",%s,%s,%u,%u,%u", + len += snprintf(&buf[len], buf_len - len, ",%s,%s,%u,%u,%u", t->test_name, dev_name, t->size, t->iteration_max, r->error); -- 2.7.4

8 years, 10 months

[PATCH v4] staging: greybus: loop_backtest: fixed consistent spacing style issue

by Jonathan Bowie

Fixed incosistent spacing around arithmetic operator. v2 -fixed subject, added changelog v3 -CC:ed linux-next incorrectly, added proper summary v4 -CC:ed staging mailing list, moved changelog to summary body Signed-off-by: Jonathan Bowie <eudjtb(a)gmail.com> --- drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..d5bb8d2 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -476,7 +476,7 @@ int format_output(struct loopback_test *t, r->gbphy_firmware_latency_jitter); } else { - len += snprintf(&buf[len], buf_len- len, ",%s,%s,%u,%u,%u", + len += snprintf(&buf[len], buf_len - len, ",%s,%s,%u,%u,%u", t->test_name, dev_name, t->size, t->iteration_max, r->error); -- 2.7.4 v2 -fixed summary and commit message

8 years, 10 months

[PATCH v3] staging: greybus: loop_backtest: fixed consistent spacing style issue

by Jonathan Bowie

Fixed incosistent spacing around arithmetic operator. Signed-off-by: Jonathan Bowie <eudjtb(a)gmail.com> --- drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..d5bb8d2 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -476,7 +476,7 @@ int format_output(struct loopback_test *t, r->gbphy_firmware_latency_jitter); } else { - len += snprintf(&buf[len], buf_len- len, ",%s,%s,%u,%u,%u", + len += snprintf(&buf[len], buf_len - len, ",%s,%s,%u,%u,%u", t->test_name, dev_name, t->size, t->iteration_max, r->error); -- 2.7.4 v2 -tried to fix summary and commit message v3 -actually fixed summary and commit message -CCed staging mailing list

8 years, 10 months

[PATCH v2] staging: greybus: loopback_test: fixed consistent spacing style issue

by Jonathan Bowie

Signed-off-by: Jonathan Bowie <eudjtb(a)gmail.com> --- v1: * Fixed inconsistent spacing around an arithmetic operator v2: * Corrected summary and commit message drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..d5bb8d2 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -476,7 +476,7 @@ int format_output(struct loopback_test *t, r->gbphy_firmware_latency_jitter); } else { - len += snprintf(&buf[len], buf_len- len, ",%s,%s,%u,%u,%u", + len += snprintf(&buf[len], buf_len - len, ",%s,%s,%u,%u,%u", t->test_name, dev_name, t->size, t->iteration_max, r->error); -- 2.7.4

8 years, 10 months

[PATCH] staging: fixed style in greybus/tools/loopback_test.c

by Jonathan Bowie

fixed a simple style issue in tools/loopback_test.c Signed-off-by: Jonathan Bowie <eudjtb(a)gmail.com> --- drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..d5bb8d2 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -476,7 +476,7 @@ int format_output(struct loopback_test *t, r->gbphy_firmware_latency_jitter); } else { - len += snprintf(&buf[len], buf_len- len, ",%s,%s,%u,%u,%u", + len += snprintf(&buf[len], buf_len - len, ",%s,%s,%u,%u,%u", t->test_name, dev_name, t->size, t->iteration_max, r->error); -- 2.7.4

8 years, 10 months

[PATCH v2 3/6] staging: greybus: Remove unnecessary braces

by simran singhal

This patch removes braces for single statement blocks. The warning was detected using checkpatch.pl. Coccinelle was used to make the change. @@ expression e,e1; @@ - if (e) { + if (e) e1; - } Signed-off-by: simran singhal <singhalsimran0(a)gmail.com> --- v2: -I forgot to cc outreachy mailing list drivers/staging/greybus/audio_manager_sysfs.c | 3 +-- drivers/staging/greybus/audio_topology.c | 9 +++------ drivers/staging/greybus/bootrom.c | 6 ++---- drivers/staging/greybus/control.c | 3 +-- drivers/staging/greybus/es2.c | 3 +-- drivers/staging/greybus/gpio.c | 6 ++---- drivers/staging/greybus/manifest.c | 3 +-- drivers/staging/greybus/module.c | 3 +-- drivers/staging/greybus/svc.c | 9 +++------ 9 files changed, 15 insertions(+), 30 deletions(-) diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c index 34ebd14..dc1d7a1 100644 --- a/drivers/staging/greybus/audio_manager_sysfs.c +++ b/drivers/staging/greybus/audio_manager_sysfs.c @@ -88,10 +88,9 @@ static void manager_sysfs_init_attribute( int err; err = sysfs_create_file(kobj, &kattr->attr); - if (err) { + if (err) pr_warn("creating the sysfs entry for %s failed: %d\n", kattr->attr.name, err); - } } void gb_audio_manager_sysfs_init(struct kobject *kobj) diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c index 3001a49..0e02f00 100644 --- a/drivers/staging/greybus/audio_topology.c +++ b/drivers/staging/greybus/audio_topology.c @@ -327,10 +327,9 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } return ret; } @@ -637,10 +636,9 @@ static int gbcodec_enum_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } return ret; } @@ -837,11 +835,10 @@ static int gbcodec_enum_dapm_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } for (wi = 0; wi < wlist->num_widgets; wi++) { widget = wlist->widgets[wi]; diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 5f90721..0e01e2b 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -176,10 +176,9 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); - if (rc) { + if (rc) dev_err(&connection->bundle->dev, "failed to find %s firmware (%d)\n", firmware_name, rc); - } return rc; } @@ -226,11 +225,10 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) mutex_unlock(&bootrom->mutex); queue_work: - if (!ret) { + if (!ret) /* Refresh timeout */ gb_bootrom_set_timeout(bootrom, NEXT_REQ_GET_FIRMWARE, NEXT_REQ_TIMEOUT_MS); - } return ret; } diff --git a/drivers/staging/greybus/control.c b/drivers/staging/greybus/control.c index 5b30be3..d8452c8 100644 --- a/drivers/staging/greybus/control.c +++ b/drivers/staging/greybus/control.c @@ -167,10 +167,9 @@ int gb_control_disconnecting_operation(struct gb_control *control, request->cport_id = cpu_to_le16(cport_id); ret = gb_operation_request_send_sync(operation); - if (ret) { + if (ret) dev_err(&control->dev, "failed to send disconnecting: %d\n", ret); - } gb_operation_put(operation); diff --git a/drivers/staging/greybus/es2.c b/drivers/staging/greybus/es2.c index 93afd93..478eb53 100644 --- a/drivers/staging/greybus/es2.c +++ b/drivers/staging/greybus/es2.c @@ -1019,10 +1019,9 @@ static int arpc_sync(struct es2_ap_dev *es2, u8 type, void *payload, spin_unlock_irqrestore(&es2->arpc_lock, flags); arpc_free(rpc); - if (retval < 0 && retval != -EREMOTEIO) { + if (retval < 0 && retval != -EREMOTEIO) dev_err(&es2->usb_dev->dev, "failed to execute ARPC: %d\n", retval); - } return retval; } diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 250caa0..ea9e3f8 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -125,10 +125,9 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *ggc, return ret; direction = response.direction; - if (direction && direction != 1) { + if (direction && direction != 1) dev_warn(dev, "gpio %u direction was %u (should be 0 or 1)\n", which, direction); - } ggc->lines[which].direction = direction ? 1 : 0; return 0; } @@ -181,10 +180,9 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *ggc, } value = response.value; - if (value && value != 1) { + if (value && value != 1) dev_warn(dev, "gpio %u value was %u (should be 0 or 1)\n", which, value); - } ggc->lines[which].value = value ? 1 : 0; return 0; } diff --git a/drivers/staging/greybus/manifest.c b/drivers/staging/greybus/manifest.c index 7b90377..eee57a3 100644 --- a/drivers/staging/greybus/manifest.c +++ b/drivers/staging/greybus/manifest.c @@ -152,11 +152,10 @@ static int identify_descriptor(struct gb_interface *intf, } /* Descriptor bigger than what we expect */ - if (desc_size > expected_size) { + if (desc_size > expected_size) dev_warn(&intf->dev, "%s descriptor size mismatch (want %zu got %zu)\n", get_descriptor_type_string(desc_header->type), expected_size, desc_size); - } descriptor = kzalloc(sizeof(*descriptor), GFP_KERNEL); if (!descriptor) diff --git a/drivers/staging/greybus/module.c b/drivers/staging/greybus/module.c index 660b467..956b8ab 100644 --- a/drivers/staging/greybus/module.c +++ b/drivers/staging/greybus/module.c @@ -148,11 +148,10 @@ static void gb_module_register_interface(struct gb_interface *intf) ret = gb_interface_activate(intf); if (ret) { - if (intf->type != GB_INTERFACE_TYPE_DUMMY) { + if (intf->type != GB_INTERFACE_TYPE_DUMMY) dev_err(&module->dev, "failed to activate interface %u: %d\n", intf_id, ret); - } gb_interface_add(intf); goto err_unlock; diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c index be151a6..dbf0f87 100644 --- a/drivers/staging/greybus/svc.c +++ b/drivers/staging/greybus/svc.c @@ -512,10 +512,9 @@ void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id, ret = gb_operation_sync(connection, GB_SVC_TYPE_CONN_DESTROY, &request, sizeof(request), NULL, 0); - if (ret) { + if (ret) dev_err(&svc->dev, "failed to destroy connection (%u:%u %u:%u): %d\n", intf1_id, cport1_id, intf2_id, cport2_id, ret); - } } /* Creates bi-directional routes between the devices */ @@ -544,10 +543,9 @@ void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id) ret = gb_operation_sync(svc->connection, GB_SVC_TYPE_ROUTE_DESTROY, &request, sizeof(request), NULL, 0); - if (ret) { + if (ret) dev_err(&svc->dev, "failed to destroy route (%u %u): %d\n", intf1_id, intf2_id, ret); - } } int gb_svc_intf_set_power_mode(struct gb_svc *svc, u8 intf_id, u8 hs_series, @@ -960,10 +958,9 @@ static void gb_svc_process_module_inserted(struct gb_operation *operation) dev_dbg(&svc->dev, "%s - id = %u, num_interfaces = %zu, flags = 0x%04x\n", __func__, module_id, num_interfaces, flags); - if (flags & GB_SVC_MODULE_INSERTED_FLAG_NO_PRIMARY) { + if (flags & GB_SVC_MODULE_INSERTED_FLAG_NO_PRIMARY) dev_warn(&svc->dev, "no primary interface detected on module %u\n", module_id); - } module = gb_svc_module_lookup(svc, module_id); if (module) { -- 2.7.4

8 years, 10 months

[PATCH 3/6] staging: greybus: Remove unnecessary braces

by simran singhal

This patch removes braces for single statement blocks. The warning was detected using checkpatch.pl. Coccinelle was used to make the change. @@ expression e,e1; @@ - if (e) { + if (e) e1; - } Signed-off-by: simran singhal <singhalsimran0(a)gmail.com> --- drivers/staging/greybus/audio_manager_sysfs.c | 3 +-- drivers/staging/greybus/audio_topology.c | 9 +++------ drivers/staging/greybus/bootrom.c | 6 ++---- drivers/staging/greybus/control.c | 3 +-- drivers/staging/greybus/es2.c | 3 +-- drivers/staging/greybus/gpio.c | 6 ++---- drivers/staging/greybus/manifest.c | 3 +-- drivers/staging/greybus/module.c | 3 +-- drivers/staging/greybus/svc.c | 9 +++------ 9 files changed, 15 insertions(+), 30 deletions(-) diff --git a/drivers/staging/greybus/audio_manager_sysfs.c b/drivers/staging/greybus/audio_manager_sysfs.c index 34ebd14..dc1d7a1 100644 --- a/drivers/staging/greybus/audio_manager_sysfs.c +++ b/drivers/staging/greybus/audio_manager_sysfs.c @@ -88,10 +88,9 @@ static void manager_sysfs_init_attribute( int err; err = sysfs_create_file(kobj, &kattr->attr); - if (err) { + if (err) pr_warn("creating the sysfs entry for %s failed: %d\n", kattr->attr.name, err); - } } void gb_audio_manager_sysfs_init(struct kobject *kobj) diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c index 3001a49..0e02f00 100644 --- a/drivers/staging/greybus/audio_topology.c +++ b/drivers/staging/greybus/audio_topology.c @@ -327,10 +327,9 @@ static int gbcodec_mixer_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } return ret; } @@ -637,10 +636,9 @@ static int gbcodec_enum_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } return ret; } @@ -837,11 +835,10 @@ static int gbcodec_enum_dapm_ctl_put(struct snd_kcontrol *kcontrol, gb_pm_runtime_put_autosuspend(bundle); - if (ret) { + if (ret) dev_err_ratelimited(codec->dev, "%d:Error in %s for %s\n", ret, __func__, kcontrol->id.name); - } for (wi = 0; wi < wlist->num_widgets; wi++) { widget = wlist->widgets[wi]; diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c index 5f90721..0e01e2b 100644 --- a/drivers/staging/greybus/bootrom.c +++ b/drivers/staging/greybus/bootrom.c @@ -176,10 +176,9 @@ static int find_firmware(struct gb_bootrom *bootrom, u8 stage) rc = request_firmware(&bootrom->fw, firmware_name, &connection->bundle->dev); - if (rc) { + if (rc) dev_err(&connection->bundle->dev, "failed to find %s firmware (%d)\n", firmware_name, rc); - } return rc; } @@ -226,11 +225,10 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op) mutex_unlock(&bootrom->mutex); queue_work: - if (!ret) { + if (!ret) /* Refresh timeout */ gb_bootrom_set_timeout(bootrom, NEXT_REQ_GET_FIRMWARE, NEXT_REQ_TIMEOUT_MS); - } return ret; } diff --git a/drivers/staging/greybus/control.c b/drivers/staging/greybus/control.c index 5b30be3..d8452c8 100644 --- a/drivers/staging/greybus/control.c +++ b/drivers/staging/greybus/control.c @@ -167,10 +167,9 @@ int gb_control_disconnecting_operation(struct gb_control *control, request->cport_id = cpu_to_le16(cport_id); ret = gb_operation_request_send_sync(operation); - if (ret) { + if (ret) dev_err(&control->dev, "failed to send disconnecting: %d\n", ret); - } gb_operation_put(operation); diff --git a/drivers/staging/greybus/es2.c b/drivers/staging/greybus/es2.c index 93afd93..478eb53 100644 --- a/drivers/staging/greybus/es2.c +++ b/drivers/staging/greybus/es2.c @@ -1019,10 +1019,9 @@ static int arpc_sync(struct es2_ap_dev *es2, u8 type, void *payload, spin_unlock_irqrestore(&es2->arpc_lock, flags); arpc_free(rpc); - if (retval < 0 && retval != -EREMOTEIO) { + if (retval < 0 && retval != -EREMOTEIO) dev_err(&es2->usb_dev->dev, "failed to execute ARPC: %d\n", retval); - } return retval; } diff --git a/drivers/staging/greybus/gpio.c b/drivers/staging/greybus/gpio.c index 250caa0..ea9e3f8 100644 --- a/drivers/staging/greybus/gpio.c +++ b/drivers/staging/greybus/gpio.c @@ -125,10 +125,9 @@ static int gb_gpio_get_direction_operation(struct gb_gpio_controller *ggc, return ret; direction = response.direction; - if (direction && direction != 1) { + if (direction && direction != 1) dev_warn(dev, "gpio %u direction was %u (should be 0 or 1)\n", which, direction); - } ggc->lines[which].direction = direction ? 1 : 0; return 0; } @@ -181,10 +180,9 @@ static int gb_gpio_get_value_operation(struct gb_gpio_controller *ggc, } value = response.value; - if (value && value != 1) { + if (value && value != 1) dev_warn(dev, "gpio %u value was %u (should be 0 or 1)\n", which, value); - } ggc->lines[which].value = value ? 1 : 0; return 0; } diff --git a/drivers/staging/greybus/manifest.c b/drivers/staging/greybus/manifest.c index 7b90377..eee57a3 100644 --- a/drivers/staging/greybus/manifest.c +++ b/drivers/staging/greybus/manifest.c @@ -152,11 +152,10 @@ static int identify_descriptor(struct gb_interface *intf, } /* Descriptor bigger than what we expect */ - if (desc_size > expected_size) { + if (desc_size > expected_size) dev_warn(&intf->dev, "%s descriptor size mismatch (want %zu got %zu)\n", get_descriptor_type_string(desc_header->type), expected_size, desc_size); - } descriptor = kzalloc(sizeof(*descriptor), GFP_KERNEL); if (!descriptor) diff --git a/drivers/staging/greybus/module.c b/drivers/staging/greybus/module.c index 660b467..956b8ab 100644 --- a/drivers/staging/greybus/module.c +++ b/drivers/staging/greybus/module.c @@ -148,11 +148,10 @@ static void gb_module_register_interface(struct gb_interface *intf) ret = gb_interface_activate(intf); if (ret) { - if (intf->type != GB_INTERFACE_TYPE_DUMMY) { + if (intf->type != GB_INTERFACE_TYPE_DUMMY) dev_err(&module->dev, "failed to activate interface %u: %d\n", intf_id, ret); - } gb_interface_add(intf); goto err_unlock; diff --git a/drivers/staging/greybus/svc.c b/drivers/staging/greybus/svc.c index be151a6..dbf0f87 100644 --- a/drivers/staging/greybus/svc.c +++ b/drivers/staging/greybus/svc.c @@ -512,10 +512,9 @@ void gb_svc_connection_destroy(struct gb_svc *svc, u8 intf1_id, u16 cport1_id, ret = gb_operation_sync(connection, GB_SVC_TYPE_CONN_DESTROY, &request, sizeof(request), NULL, 0); - if (ret) { + if (ret) dev_err(&svc->dev, "failed to destroy connection (%u:%u %u:%u): %d\n", intf1_id, cport1_id, intf2_id, cport2_id, ret); - } } /* Creates bi-directional routes between the devices */ @@ -544,10 +543,9 @@ void gb_svc_route_destroy(struct gb_svc *svc, u8 intf1_id, u8 intf2_id) ret = gb_operation_sync(svc->connection, GB_SVC_TYPE_ROUTE_DESTROY, &request, sizeof(request), NULL, 0); - if (ret) { + if (ret) dev_err(&svc->dev, "failed to destroy route (%u %u): %d\n", intf1_id, intf2_id, ret); - } } int gb_svc_intf_set_power_mode(struct gb_svc *svc, u8 intf_id, u8 hs_series, @@ -960,10 +958,9 @@ static void gb_svc_process_module_inserted(struct gb_operation *operation) dev_dbg(&svc->dev, "%s - id = %u, num_interfaces = %zu, flags = 0x%04x\n", __func__, module_id, num_interfaces, flags); - if (flags & GB_SVC_MODULE_INSERTED_FLAG_NO_PRIMARY) { + if (flags & GB_SVC_MODULE_INSERTED_FLAG_NO_PRIMARY) dev_warn(&svc->dev, "no primary interface detected on module %u\n", module_id); - } module = gb_svc_module_lookup(svc, module_id); if (module) { -- 2.7.4

8 years, 10 months

[PATCH] greybus: fw-management: Replace strncpy with strlcpy

by Tobin C. Harding

Greybus currently uses strncpy() coupled with a check for '\0' on the last byte of various buffers. strncpy() is passed size parameter equal to the size of the buffer in all instances. If the source string is larger than the destination buffer the check catches this and, after logging the error, returns an error value. In one instance the immediate return is not required. Using strncpy() with the manual check adds code that could be removed by the use of strlcpy(), although truncation then needs to be checked. Replace calls to strncpy() with calls to strlcpy(). Replace null termination checks with checks for truncated string. Add log message if string is truncated but do not return an error code. Signed-off-by: Tobin C. Harding <me(a)tobin.cc> --- drivers/staging/greybus/fw-management.c | 59 +++++++++++---------------------- 1 file changed, 19 insertions(+), 40 deletions(-) diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c index 3cd6cf0..1cd5a45 100644 --- a/drivers/staging/greybus/fw-management.c +++ b/drivers/staging/greybus/fw-management.c @@ -108,6 +108,7 @@ static int fw_mgmt_interface_fw_version_operation(struct fw_mgmt *fw_mgmt, struct gb_connection *connection = fw_mgmt->connection; struct gb_fw_mgmt_interface_fw_version_response response; int ret; + size_t len; ret = gb_operation_sync(connection, GB_FW_MGMT_TYPE_INTERFACE_FW_VERSION, NULL, 0, @@ -121,18 +122,11 @@ static int fw_mgmt_interface_fw_version_operation(struct fw_mgmt *fw_mgmt, fw_info->major = le16_to_cpu(response.major); fw_info->minor = le16_to_cpu(response.minor); - strncpy(fw_info->firmware_tag, response.firmware_tag, + len = strlcpy(fw_info->firmware_tag, response.firmware_tag, GB_FIRMWARE_TAG_MAX_SIZE); - - /* - * The firmware-tag should be NULL terminated, otherwise throw error but - * don't fail. - */ - if (fw_info->firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') { + if (len >= GB_FIRMWARE_TAG_MAX_SIZE) dev_err(fw_mgmt->parent, - "fw-version: firmware-tag is not NULL terminated\n"); - fw_info->firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0'; - } + "fw-version: firmware-tag has been truncated\n"); return 0; } @@ -142,6 +136,7 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt, { struct gb_fw_mgmt_load_and_validate_fw_request request; int ret; + size_t len; if (load_method != GB_FW_LOAD_METHOD_UNIPRO && load_method != GB_FW_LOAD_METHOD_INTERNAL) { @@ -151,16 +146,10 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt, } request.load_method = load_method; - strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE); - - /* - * The firmware-tag should be NULL terminated, otherwise throw error and - * fail. - */ - if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') { - dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n"); - return -EINVAL; - } + len = strlcpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE); + if (len >= GB_FIRMWARE_TAG_MAX_SIZE) + dev_err(fw_mgmt->parent, + "load-and-validate: firmware-tag has been truncated\n"); /* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */ ret = ida_simple_get(&fw_mgmt->id_map, 1, 256, GFP_KERNEL); @@ -247,18 +236,13 @@ static int fw_mgmt_backend_fw_version_operation(struct fw_mgmt *fw_mgmt, struct gb_fw_mgmt_backend_fw_version_request request; struct gb_fw_mgmt_backend_fw_version_response response; int ret; + size_t len; - strncpy(request.firmware_tag, fw_info->firmware_tag, + len = strlcpy(request.firmware_tag, fw_info->firmware_tag, GB_FIRMWARE_TAG_MAX_SIZE); - - /* - * The firmware-tag should be NULL terminated, otherwise throw error and - * fail. - */ - if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') { - dev_err(fw_mgmt->parent, "backend-version: firmware-tag is not NULL terminated\n"); - return -EINVAL; - } + if (len >= GB_FIRMWARE_TAG_MAX_SIZE) + dev_err(fw_mgmt->parent, + "backend-version: firmware-tag has been truncated\n"); ret = gb_operation_sync(connection, GB_FW_MGMT_TYPE_BACKEND_FW_VERSION, &request, @@ -301,17 +285,12 @@ static int fw_mgmt_backend_fw_update_operation(struct fw_mgmt *fw_mgmt, { struct gb_fw_mgmt_backend_fw_update_request request; int ret; + size_t len; - strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE); - - /* - * The firmware-tag should be NULL terminated, otherwise throw error and - * fail. - */ - if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') { - dev_err(fw_mgmt->parent, "backend-update: firmware-tag is not NULL terminated\n"); - return -EINVAL; - } + len = strlcpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE); + if (len >= GB_FIRMWARE_TAG_MAX_SIZE) + dev_err(fw_mgmt->parent, + "backend-update: firmware-tag has been truncated\n"); /* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */ ret = ida_simple_get(&fw_mgmt->id_map, 1, 256, GFP_KERNEL); -- 2.7.4

8 years, 10 months

[PATCH v2] staging: greybus: loopback_test: Fix open error path

by sayli karnik

Change array index from the loop bound variable to loop index. If a poll file fails to open for any intermediate device, all poll files with fds of devices from 0 upto that device must be closed in the open_poll_files() function. The current code only closes the poll file with the most recent fd allocated, and at times tries to close the same file multiple times. Detected by coccinelle: @@ expression arr,ex1,ex2; @@ for(ex1 = 0; ex1 < ex2; ex1++) { <... arr[ - ex2 + ex1 ] ...> } Signed-off-by: sayli karnik <karniksayli1995(a)gmail.com> --- v2: Made the subject and changelog more concise drivers/staging/greybus/tools/loopback_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index 18d7a3d..2ee9a22 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -674,7 +674,7 @@ static int open_poll_files(struct loopback_test *t) err: for (i = 0; i < fds_idx; i++) - close(t->fds[fds_idx].fd); + close(t->fds[i].fd); return -1; } -- 2.7.4

8 years, 10 months

Re: [greybus-dev] [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index

by Johan Hovold

[+CC: greybus and staging lists] On Sat, Feb 18, 2017 at 01:47:38PM +0530, sayli karnik wrote: > Change array index from the loop bound variable to loop index. > The open_poll_files() functions attempts to open poll files of devices > numbered from 0 to device_count. If the open() function inside it is > unsuccessful for any intermediate device, all files with fds of devices from 0 > upto that device must be closed and open_poll_files() should return -1. > The current code only closes the poll file with the most recent fd allocated, > and in most cases tries to close the same file multiple times. Nice catch! You forgot to CC the relevant mailings lists however (use scripts/get_maintainer.pl). Also your patch summary is a bit too detailed, something like staging: greybus: loopback_test: fix open error path would be better. Care to resend as a v2 with a shorter summary and lists on CC? > Detected by coccinelle: > > @@ > expression arr,ex1,ex2; > @@ > > for(ex1 = 0; ex1 < ex2; ex1++) { <... > arr[ > - ex2 > + ex1 > ] > ...> } > > Signed-off-by: sayli karnik <karniksayli1995(a)gmail.com> > --- > drivers/staging/greybus/tools/loopback_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c > index 18d7a3d..2ee9a22 100644 > --- a/drivers/staging/greybus/tools/loopback_test.c > +++ b/drivers/staging/greybus/tools/loopback_test.c > @@ -674,7 +674,7 @@ static int open_poll_files(struct loopback_test *t) > > err: > for (i = 0; i < fds_idx; i++) > - close(t->fds[fds_idx].fd); > + close(t->fds[i].fd); > > return -1; > } Thanks, Johan

8 years, 10 months

← Newer
1
...
60
61
62
63
64
65
66
67
68
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

greybus-dev