From: Guenter Roeck <linux@roeck-us.net> To: Greg KH <gregkh@linuxfoundation.org> Cc: Kyle Tso <kyletso@google.com>, heikki.krogerus@linux.intel.com, badhri@google.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] usb: typec: tcpm: Export partner Source Capabilities Date: Thu, 31 Jan 2019 10:22:13 -0800 [thread overview] Message-ID: <20190131182213.GB17285@roeck-us.net> (raw) In-Reply-To: <20190131070238.GA4500@kroah.com> On Thu, Jan 31, 2019 at 08:02:38AM +0100, Greg KH wrote: > On Thu, Jan 31, 2019 at 11:54:11AM +0800, Kyle Tso wrote: > > Provide a function to get the partner Source Capabilities. > > > > Signed-off-by: Kyle Tso <kyletso@google.com> > > --- > > drivers/usb/typec/tcpm/tcpm.c | 23 +++++++++++++++++++++++ > > include/linux/usb/tcpm.h | 1 + > > 2 files changed, 24 insertions(+) > > > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > > index f1d3e54210df..29cd84ba9960 100644 > > --- a/drivers/usb/typec/tcpm/tcpm.c > > +++ b/drivers/usb/typec/tcpm/tcpm.c > > @@ -4494,6 +4494,29 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo, > > } > > EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities); > > > > +/* > > + * Don't call this function in interrupt context. Caller needs to free the > > + * memory itself. > > + */ > > +int tcpm_get_partner_src_caps(struct tcpm_port *port, u32 **src_pdo) > > +{ > > + unsigned int nr_pdo; > > + > > + if (port->nr_source_caps == 0) > > + return -ENODATA; > > + > > + *src_pdo = kcalloc(port->nr_source_caps, sizeof(u32), GFP_KERNEL); > > + if (!src_pdo) > > + return -ENOMEM; > > + > > + mutex_lock(&port->lock); > > + nr_pdo = tcpm_copy_pdos(*src_pdo, port->source_caps, > > + port->nr_source_caps); > > + mutex_unlock(&port->lock); The mutex use here suggests that the data can be updated while being copied. But that suggests that port->nr_source_caps can change as well and may no longer be current after mutex_lock(). > > + return nr_pdo; > > +} > > +EXPORT_SYMBOL_GPL(tcpm_get_partner_src_caps); > > We don't add new functions that no one uses :( > I am also concerned about the API itself; passing a pointer to be freed by the caller invites memory leaks. Guenter
WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <linux@roeck-us.net> To: Greg KH <gregkh@linuxfoundation.org> Cc: Kyle Tso <kyletso@google.com>, heikki.krogerus@linux.intel.com, badhri@google.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: usb: typec: tcpm: Export partner Source Capabilities Date: Thu, 31 Jan 2019 10:22:13 -0800 [thread overview] Message-ID: <20190131182213.GB17285@roeck-us.net> (raw) On Thu, Jan 31, 2019 at 08:02:38AM +0100, Greg KH wrote: > On Thu, Jan 31, 2019 at 11:54:11AM +0800, Kyle Tso wrote: > > Provide a function to get the partner Source Capabilities. > > > > Signed-off-by: Kyle Tso <kyletso@google.com> > > --- > > drivers/usb/typec/tcpm/tcpm.c | 23 +++++++++++++++++++++++ > > include/linux/usb/tcpm.h | 1 + > > 2 files changed, 24 insertions(+) > > > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > > index f1d3e54210df..29cd84ba9960 100644 > > --- a/drivers/usb/typec/tcpm/tcpm.c > > +++ b/drivers/usb/typec/tcpm/tcpm.c > > @@ -4494,6 +4494,29 @@ int tcpm_update_sink_capabilities(struct tcpm_port *port, const u32 *pdo, > > } > > EXPORT_SYMBOL_GPL(tcpm_update_sink_capabilities); > > > > +/* > > + * Don't call this function in interrupt context. Caller needs to free the > > + * memory itself. > > + */ > > +int tcpm_get_partner_src_caps(struct tcpm_port *port, u32 **src_pdo) > > +{ > > + unsigned int nr_pdo; > > + > > + if (port->nr_source_caps == 0) > > + return -ENODATA; > > + > > + *src_pdo = kcalloc(port->nr_source_caps, sizeof(u32), GFP_KERNEL); > > + if (!src_pdo) > > + return -ENOMEM; > > + > > + mutex_lock(&port->lock); > > + nr_pdo = tcpm_copy_pdos(*src_pdo, port->source_caps, > > + port->nr_source_caps); > > + mutex_unlock(&port->lock); The mutex use here suggests that the data can be updated while being copied. But that suggests that port->nr_source_caps can change as well and may no longer be current after mutex_lock(). > > + return nr_pdo; > > +} > > +EXPORT_SYMBOL_GPL(tcpm_get_partner_src_caps); > > We don't add new functions that no one uses :( > I am also concerned about the API itself; passing a pointer to be freed by the caller invites memory leaks. Guenter
next prev parent reply other threads:[~2019-01-31 18:22 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-31 3:54 [PATCH] usb: typec: tcpm: Export partner Source Capabilities Kyle Tso 2019-01-31 3:54 ` Kyle Tso 2019-01-31 7:02 ` [PATCH] " Greg KH 2019-01-31 7:02 ` Greg Kroah-Hartman 2019-01-31 18:22 ` Guenter Roeck [this message] 2019-01-31 18:22 ` Guenter Roeck 2019-02-12 10:33 ` [PATCH] " Kyle Tso 2019-02-12 10:33 ` Kyle Tso 2019-02-12 10:32 ` [PATCH] " Kyle Tso 2019-02-12 10:32 ` Kyle Tso [not found] ` <CAGZ6i=07EOHqd3r9y1MpYn_e1nFeYGt_wp9arhMZWWeTBm-udA@mail.gmail.com> 2019-02-12 10:54 ` [PATCH] " Greg KH 2019-02-12 10:54 ` Greg Kroah-Hartman 2019-02-12 16:19 ` [PATCH] " Guenter Roeck 2019-02-12 16:19 ` Guenter Roeck 2019-02-14 14:17 ` [PATCH] " Adam Thomson 2019-02-14 14:17 ` Opensource [Adam Thomson] 2019-02-14 14:30 ` [PATCH] " Greg KH 2019-02-14 14:30 ` Greg Kroah-Hartman 2019-02-14 15:28 ` [PATCH] " Adam Thomson 2019-02-14 15:28 ` Opensource [Adam Thomson] 2019-02-14 14:31 ` [PATCH] " Guenter Roeck 2019-02-14 14:31 ` Guenter Roeck
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190131182213.GB17285@roeck-us.net \ --to=linux@roeck-us.net \ --cc=badhri@google.com \ --cc=gregkh@linuxfoundation.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=kyletso@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.