* [PATCH BlueZ v2 1/1] mesh: Fixed warning-to-error from GCC 8.1.1
@ 2018-08-23 20:08 Brian Gix
2018-08-24 7:48 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Brian Gix @ 2018-08-23 20:08 UTC (permalink / raw)
To: linux-bluetooth
Cc: marcel, johan.hedberg, inga.stotland, johan.hedberg,
luiz.von.dentz, Brian Gix
Fixed compiler flagged unsafe usage of strncpy
---
mesh/storage.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/mesh/storage.c b/mesh/storage.c
index 85fa81dda..937f801a6 100644
--- a/mesh/storage.c
+++ b/mesh/storage.c
@@ -298,13 +298,11 @@ bool storage_parse_config(struct mesh_net *net, const char *config_name)
result = parse_config(net, config_name, false);
if (!result) {
- char *bak = (char *) l_malloc(strlen(config_name) + 5);
-
- if (!bak)
- goto done;
+ size_t len = strlen(config_name) + 5;
+ char *bak = l_malloc(len);
/* Fall-back to Backup version */
- strncpy(bak, config_name, strlen(config_name) + 1);
+ strncpy(bak, config_name, len);
bak = strncat(bak, ".bak", 5);
remove(config_name);
@@ -592,15 +590,13 @@ struct write_info {
static void idle_save_config(void *user_data)
{
struct write_info *info = user_data;
- char *tmp = (char *) l_malloc(strlen(info->config_name) + 5);
- char *bak = (char *) l_malloc(strlen(info->config_name) + 5);
+ size_t len = strlen(info->config_name) + 5;
+ char *tmp = l_malloc(len);
+ char *bak = l_malloc(len);
bool result = false;
- if (!tmp || !bak)
- goto done;
-
- strncpy(tmp, info->config_name, strlen(info->config_name) + 1);
- strncpy(bak, info->config_name, strlen(info->config_name) + 1);
+ strncpy(tmp, info->config_name, len);
+ strncpy(bak, info->config_name, len);
tmp = strncat(tmp, ".tmp", 5);
bak = strncat(bak, ".bak", 5);
remove(tmp);
@@ -615,7 +611,6 @@ static void idle_save_config(void *user_data)
}
remove(tmp);
-done:
l_free(tmp);
l_free(bak);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH BlueZ v2 1/1] mesh: Fixed warning-to-error from GCC 8.1.1
2018-08-23 20:08 [PATCH BlueZ v2 1/1] mesh: Fixed warning-to-error from GCC 8.1.1 Brian Gix
@ 2018-08-24 7:48 ` Luiz Augusto von Dentz
2018-08-24 14:52 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2018-08-24 7:48 UTC (permalink / raw)
To: Brian Gix
Cc: linux-bluetooth@vger.kernel.org, Marcel Holtmann, Johan Hedberg,
Stotland, Inga, Johan Hedberg, Luiz Augusto Von Dentz
Hi Brian,
On Thu, Aug 23, 2018 at 11:08 PM, Brian Gix <brian.gix@intel.com> wrote:
> Fixed compiler flagged unsafe usage of strncpy
> ---
> mesh/storage.c | 21 ++++++++-------------
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/mesh/storage.c b/mesh/storage.c
> index 85fa81dda..937f801a6 100644
> --- a/mesh/storage.c
> +++ b/mesh/storage.c
> @@ -298,13 +298,11 @@ bool storage_parse_config(struct mesh_net *net, const char *config_name)
> result = parse_config(net, config_name, false);
>
> if (!result) {
> - char *bak = (char *) l_malloc(strlen(config_name) + 5);
> -
> - if (!bak)
> - goto done;
> + size_t len = strlen(config_name) + 5;
> + char *bak = l_malloc(len);
I would consider allocating bak in the stack, or perhaps use alloca if
you don't want to waste stack space, that way we don't have to free
these string that are just used temporarily.
>
> /* Fall-back to Backup version */
> - strncpy(bak, config_name, strlen(config_name) + 1);
> + strncpy(bak, config_name, len);
> bak = strncat(bak, ".bak", 5);
>
> remove(config_name);
> @@ -592,15 +590,13 @@ struct write_info {
> static void idle_save_config(void *user_data)
> {
> struct write_info *info = user_data;
> - char *tmp = (char *) l_malloc(strlen(info->config_name) + 5);
> - char *bak = (char *) l_malloc(strlen(info->config_name) + 5);
> + size_t len = strlen(info->config_name) + 5;
> + char *tmp = l_malloc(len);
> + char *bak = l_malloc(len);
Same here.
> bool result = false;
>
> - if (!tmp || !bak)
> - goto done;
> -
> - strncpy(tmp, info->config_name, strlen(info->config_name) + 1);
> - strncpy(bak, info->config_name, strlen(info->config_name) + 1);
> + strncpy(tmp, info->config_name, len);
> + strncpy(bak, info->config_name, len);
> tmp = strncat(tmp, ".tmp", 5);
> bak = strncat(bak, ".bak", 5);
> remove(tmp);
> @@ -615,7 +611,6 @@ static void idle_save_config(void *user_data)
> }
>
> remove(tmp);
> -done:
> l_free(tmp);
> l_free(bak);
>
> --
> 2.17.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH BlueZ v2 1/1] mesh: Fixed warning-to-error from GCC 8.1.1
2018-08-24 7:48 ` Luiz Augusto von Dentz
@ 2018-08-24 14:52 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2018-08-24 14:52 UTC (permalink / raw)
To: Brian Gix
Cc: linux-bluetooth@vger.kernel.org, Marcel Holtmann, Johan Hedberg,
Stotland, Inga, Johan Hedberg, Luiz Augusto Von Dentz
Hi Brian,
On Fri, Aug 24, 2018 at 10:48 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Brian,
>
> On Thu, Aug 23, 2018 at 11:08 PM, Brian Gix <brian.gix@intel.com> wrote:
>> Fixed compiler flagged unsafe usage of strncpy
>> ---
>> mesh/storage.c | 21 ++++++++-------------
>> 1 file changed, 8 insertions(+), 13 deletions(-)
>>
>> diff --git a/mesh/storage.c b/mesh/storage.c
>> index 85fa81dda..937f801a6 100644
>> --- a/mesh/storage.c
>> +++ b/mesh/storage.c
>> @@ -298,13 +298,11 @@ bool storage_parse_config(struct mesh_net *net, const char *config_name)
>> result = parse_config(net, config_name, false);
>>
>> if (!result) {
>> - char *bak = (char *) l_malloc(strlen(config_name) + 5);
>> -
>> - if (!bak)
>> - goto done;
>> + size_t len = strlen(config_name) + 5;
>> + char *bak = l_malloc(len);
>
> I would consider allocating bak in the stack, or perhaps use alloca if
> you don't want to waste stack space, that way we don't have to free
> these string that are just used temporarily.
>
>>
>> /* Fall-back to Backup version */
>> - strncpy(bak, config_name, strlen(config_name) + 1);
>> + strncpy(bak, config_name, len);
>> bak = strncat(bak, ".bak", 5);
>>
>> remove(config_name);
>> @@ -592,15 +590,13 @@ struct write_info {
>> static void idle_save_config(void *user_data)
>> {
>> struct write_info *info = user_data;
>> - char *tmp = (char *) l_malloc(strlen(info->config_name) + 5);
>> - char *bak = (char *) l_malloc(strlen(info->config_name) + 5);
>> + size_t len = strlen(info->config_name) + 5;
>> + char *tmp = l_malloc(len);
>> + char *bak = l_malloc(len);
>
> Same here.
>
>> bool result = false;
>>
>> - if (!tmp || !bak)
>> - goto done;
>> -
>> - strncpy(tmp, info->config_name, strlen(info->config_name) + 1);
>> - strncpy(bak, info->config_name, strlen(info->config_name) + 1);
>> + strncpy(tmp, info->config_name, len);
>> + strncpy(bak, info->config_name, len);
>> tmp = strncat(tmp, ".tmp", 5);
>> bak = strncat(bak, ".bak", 5);
>> remove(tmp);
>> @@ -615,7 +611,6 @@ static void idle_save_config(void *user_data)
>> }
>>
>> remove(tmp);
>> -done:
>> l_free(tmp);
>> l_free(bak);
>>
>> --
>> 2.17.1
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-24 14:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-23 20:08 [PATCH BlueZ v2 1/1] mesh: Fixed warning-to-error from GCC 8.1.1 Brian Gix
2018-08-24 7:48 ` Luiz Augusto von Dentz
2018-08-24 14:52 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).