Re: [PATCH 4/4] alfred: introduce 'server status' IPC call

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

On Sunday, 2 January 2022 12:31:36 CET Marek Lindner wrote:
> +       list_for_each_entry(interface, &globals->interfaces, list) {
> +               strncpy(status_rep->ifaces[iface_count].name,
> +                       interface->interface,
> +                       sizeof(status_rep->ifaces[iface_count].name));
> +
> +               iface_count++;
> +       }
> +
> +       strncpy(status_rep->bat_iface, globals->mesh_iface,
> +               sizeof(status_rep->bat_iface));

strncpy doesn't guarantee that the copied string ends with a \0. You have to 
take care of that yourself.

This might not be a big problem here because the name buffers in the 
interfaces list have currently the same size as the name buffers in 
status_rep. Still feels better to have the line to manually set \0 at
the end of the buffer.


Another thing I've just noticed: You don't take care of initialization all the 
bytes in the returned buffer. So you might leak data from the alfred process's 
stack to the client.


@Simon, would you prefer to have a global "status" message (which cannot be 
extended in the future) or separate "GET" queries for the existing commands:

* ALFRED_MODESWITCH -> ALFRED_GET_MODE
* ALFRED_CHANGE_INTERFACE -> ALFRED_GET_INTERFACES
* ALFRED_CHANGE_BAT_IFACE -> ALFRED_GET_BAT_IFACE

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]