On Mon, May 30, 2022 at 09:34:15PM +0200, Arkadiusz Kubalewski wrote:
> Used by synce_dev to interact with its ports.
> +struct synce_port *synce_port_create(const char *port_name)
> +{
> + struct synce_port *p = NULL;
> +
> + if (!port_name) {
> + pr_err("%s failed - port_name not provided", __func__);
> + return NULL;
> + }
> +
> + p = malloc(sizeof(struct synce_port));
> + if (!p) {
> + pr_err("%s failed", __func__);
> + return NULL;
> + }
> + memcpy(p->name, port_name, sizeof(p->name));
> + p->state = PORT_CREATED;
> +
> + return p;
> +}
This leaves some fields of synce_port uninitialized (e.g. port->pc).
With some specific failures in synce_port_init(), synce_port_destroy()
will get the uninitialized data and do unexpected things.
Maybe the create and init function could be merged to make the
error handling easier?
> +int synce_port_enable_recover_clock(struct synce_port *port)
> +{
> + if (!port) {
> + pr_err("%s port is NULL", __func__);
> + return -EINVAL;
> + }
> +
> + if (!port->recover_clock_disable_cmd) {
I think this should be recover_clock_enable_cmd.
> + pr_err("recover_clock_enable_cmd is null on %s", port->name);
> + return -EINVAL;
> + }
--
Miroslav Lichvar
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
