-----Original Message-----
From: Miroslav Lichvar <[email protected]>
Sent: Wednesday, June 1, 2022 11:39 AM
> 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.
That is true, great catch!
>
> Maybe the create and init function could be merged to make the
> error handling easier?
Well, init is already long function, I would rather leave it separated,
Isn't it enough to zero-memory here?
>
> > +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.
Yes, another catch!
Thank you,
Arkadiusz
>
> > + pr_err("recover_clock_enable_cmd is null on %s", port->name);
> > + return -EINVAL;
> > + }
>
> --
> Miroslav Lichvar
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
