[PATCH 06/14] Cleanup: ensure string is 0 terminated

[Dirk Hohndel]

> On Jan 3, 2018, at 7:23 AM, Jef Driesen <jef at libdivecomputer.org> wrote:
> 
> On 29-12-17 01:35, Dirk Hohndel wrote:
>> Coverity CID 207790
>> Signed-off-by: Dirk Hohndel <dirk at hohndel.org>
>> ---
>>  src/irda.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/src/irda.c b/src/irda.c
>> index 149808aaa5c2..2964d4d8cb3a 100644
>> --- a/src/irda.c
>> +++ b/src/irda.c
>> @@ -229,10 +229,12 @@ dc_irda_connect_name (dc_iostream_t *abstract, unsigned int address, const char
>>  	struct sockaddr_irda peer;
>>  	peer.sir_family = AF_IRDA;
>>  	peer.sir_addr = address;
>> -	if (name)
>> +	if (name) {
>>  		strncpy (peer.sir_name, name, 25);
>> -	else
>> +		peer.sir_name[24] = '\0';
>> +	} else {
>>  		memset (peer.sir_name, 0x00, 25);
>> +	}
>>  #endif
> 
> For this one, I'm not sure whether your fix is the right thing to do. If the sir_name field is used as a fixed size byte array (possibly padded with zero's) instead of a null terminated string, then null terminating it may silently truncate the last byte and cause problems. I don't know how the irda stack uses this field, so I simply don't know the right answer here.

That's the same worry I had.
> 
> Checking for possible truncation, and returning an error if the string doesn't fit, is probably a safer solution. Even without this patch, truncation is already a problem.
> 
> PS: Using sizeof instead of hardcoding the length to 25 would be a good idea as well.

I'll do some more research on what the IRDA stack expects here and come up with a better fix.

Thanks for the review

/D