[PATCH 1/1] mesh: Fixed warning-to-error from GCC 8.1.1

[PATCH 1/1] mesh: Fixed warning-to-error from GCC 8.1.1

[Brian Gix]

Fixed compiler flagged unsafe usage of strncpy
---
 mesh/storage.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mesh/storage.c b/mesh/storage.c
index 85fa81dda..1be403297 100644
--- a/mesh/storage.c
+++ b/mesh/storage.c
@@ -298,13 +298,14 @@ 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);
+		size_t len = strlen(config_name) + 5;
+		char *bak = (char *) l_malloc(len);
 
 		if (!bak)
 			goto done;
 
 		/* 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 +593,16 @@ 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 = (char *) l_malloc(len);
+	char *bak = (char *) 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);
-- 
2.17.1

Re: [PATCH 1/1] mesh: Fixed warning-to-error from GCC 8.1.1

[Marcel Holtmann]

Hi Brian,

> Fixed compiler flagged unsafe usage of strncpy
> ---
> mesh/storage.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/mesh/storage.c b/mesh/storage.c
> index 85fa81dda..1be403297 100644
> --- a/mesh/storage.c
> +++ b/mesh/storage.c
> @@ -298,13 +298,14 @@ 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);
> +		size_t len = strlen(config_name) + 5;
> +		char *bak = (char *) l_malloc(len);

The (char *) cast is utterly pointless since l_malloc return void *.

> 
> 		if (!bak)
> 			goto done;

So why are we still doing? l_malloc will abort if no memory can be allocated. We need to stop dealing with memory allocation error handling. If you need that large memory then malloc should be used. If you use l_malloc then expect the daemon might be killed. Which is fine systemd can restart it.

Regards

Marcel

Re: [PATCH 1/1] mesh: Fixed warning-to-error from GCC 8.1.1

[Gix, Brian]

QWRkcmVzc2VkIGFuZCBwYXRjaCByZWdlbmVyYXRlZA0KDQpPbiBUaHUsIDIwMTgtMDgtMjMgYXQg
MjE6NTcgKzAyMDAsIE1hcmNlbCBIb2x0bWFubiB3cm90ZToNCj4gSGkgQnJpYW4sDQo+IA0KPiA+
IEZpeGVkIGNvbXBpbGVyIGZsYWdnZWQgdW5zYWZlIHVzYWdlIG9mIHN0cm5jcHkNCj4gPiAtLS0N
Cj4gPiBtZXNoL3N0b3JhZ2UuYyB8IDE0ICsrKysrKysrLS0tLS0tDQo+ID4gMSBmaWxlIGNoYW5n
ZWQsIDggaW5zZXJ0aW9ucygrKSwgNiBkZWxldGlvbnMoLSkNCj4gPiANCj4gPiBkaWZmIC0tZ2l0
IGEvbWVzaC9zdG9yYWdlLmMgYi9tZXNoL3N0b3JhZ2UuYw0KPiA+IGluZGV4IDg1ZmE4MWRkYS4u
MWJlNDAzMjk3IDEwMDY0NA0KPiA+IC0tLSBhL21lc2gvc3RvcmFnZS5jDQo+ID4gKysrIGIvbWVz
aC9zdG9yYWdlLmMNCj4gPiBAQCAtMjk4LDEzICsyOTgsMTQgQEAgYm9vbCBzdG9yYWdlX3BhcnNl
X2NvbmZpZyhzdHJ1Y3QgbWVzaF9uZXQgKm5ldCwgY29uc3QgY2hhciAqY29uZmlnX25hbWUpDQo+
ID4gCXJlc3VsdCA9IHBhcnNlX2NvbmZpZyhuZXQsIGNvbmZpZ19uYW1lLCBmYWxzZSk7DQo+ID4g
DQo+ID4gCWlmICghcmVzdWx0KSB7DQo+ID4gLQkJY2hhciAqYmFrID0gKGNoYXIgKikgbF9tYWxs
b2Moc3RybGVuKGNvbmZpZ19uYW1lKSArIDUpOw0KPiA+ICsJCXNpemVfdCBsZW4gPSBzdHJsZW4o
Y29uZmlnX25hbWUpICsgNTsNCj4gPiArCQljaGFyICpiYWsgPSAoY2hhciAqKSBsX21hbGxvYyhs
ZW4pOw0KPiANCj4gVGhlIChjaGFyICopIGNhc3QgaXMgdXR0ZXJseSBwb2ludGxlc3Mgc2luY2Ug
bF9tYWxsb2MgcmV0dXJuIHZvaWQgKi4NCj4gDQo+ID4gDQo+ID4gCQlpZiAoIWJhaykNCj4gPiAJ
CQlnb3RvIGRvbmU7DQo+IA0KPiBTbyB3aHkgYXJlIHdlIHN0aWxsIGRvaW5nPyBsX21hbGxvYyB3
aWxsIGFib3J0IGlmIG5vIG1lbW9yeSBjYW4gYmUgYWxsb2NhdGVkLiBXZSBuZWVkIHRvIHN0b3Ag
ZGVhbGluZyB3aXRoDQo+IG1lbW9yeSBhbGxvY2F0aW9uIGVycm9yIGhhbmRsaW5nLiBJZiB5b3Ug
bmVlZCB0aGF0IGxhcmdlIG1lbW9yeSB0aGVuIG1hbGxvYyBzaG91bGQgYmUgdXNlZC4gSWYgeW91
IHVzZQ0KPiBsX21hbGxvYyB0aGVuIGV4cGVjdCB0aGUgZGFlbW9uIG1pZ2h0IGJlIGtpbGxlZC4g
V2hpY2ggaXMgZmluZSBzeXN0ZW1kIGNhbiByZXN0YXJ0IGl0Lg0KPiANCj4gUmVnYXJkcw0KPiAN
Cj4gTWFyY2VsDQo+IA==