The function converts strings like ttyS0 and ttyUSB0 to dev_t like
(4, 64) and (188, 0). It does this by scanning tty_drivers list for
corresponding device name and index. If the driver is not registered,
this function returns -ENODEV. It also acquires tty_mutex.
Signed-off-by: Okash Khawaja <[email protected]>
---
drivers/tty/tty_io.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/tty.h | 4 ++++
2 files changed, 49 insertions(+)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -325,6 +325,51 @@ static struct tty_driver *get_tty_driver
return NULL;
}
+/**
+ * tty_dev_name_to_number - return dev_t for device name
+ * @device_name: user space name of device under /dev
+ * @dev_no: pointer to dev_t that this function will populate
+ *
+ * This function converts device names like ttyS0 or ttyUSB1 into dev_t
+ * like (4, 64) or (188, 1). If no corresponding driver is registered then
+ * the function returns -ENODEV.
+ *
+ * Locking: this acquires tty_mutex
+ */
+int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
+{
+ struct tty_driver *p;
+ int rv, index, prefix_length = 0;
+
+ while (!isdigit(*(dev_name + prefix_length)) && prefix_length <
+ strlen(dev_name) )
+ prefix_length++;
+
+ if (prefix_length == strlen(dev_name))
+ return -EINVAL;
+
+ mutex_lock(&tty_mutex);
+
+ list_for_each_entry(p, &tty_drivers, tty_drivers)
+ if (prefix_length == strlen(p->name) && strncmp(dev_name,
+ p->name, prefix_length) == 0) {
+ rv = kstrtoint(dev_name + prefix_length, 10, &index);
+ if (rv) {
+ mutex_unlock(&tty_mutex);
+ return rv;
+ }
+ if (index < p->num) {
+ *dev_no = MKDEV(p->major, p->minor_start + index);
+ mutex_unlock(&tty_mutex);
+ return 0;
+ }
+ }
+
+ mutex_unlock(&tty_mutex);
+ return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(tty_dev_name_to_number);
+
#ifdef CONFIG_CONSOLE_POLL
/**
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -401,6 +401,7 @@ extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
struct file *filp);
+extern int tty_dev_name_to_number(char *dev_name, dev_t *dev_no);
#else
static inline void tty_kref_put(struct tty_struct *tty)
{ }
@@ -424,6 +425,9 @@ static inline const char *tty_name(const
static inline struct tty_struct *tty_open_by_driver(dev_t device,
struct inode *inode, struct file *filp)
{ return NULL; }
+static inline int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
+{ return -ENOTSUPP; }
+
#endif
extern struct ktermios tty_std_termios;
On Sun, Jun 18, 2017 at 11:58 AM, Okash Khawaja <[email protected]> wrote:
> The function converts strings like ttyS0 and ttyUSB0 to dev_t like
> (4, 64) and (188, 0). It does this by scanning tty_drivers list for
> corresponding device name and index. If the driver is not registered,
> this function returns -ENODEV. It also acquires tty_mutex.
Nice!
Few comments below.
> +/**
> + * tty_dev_name_to_number - return dev_t for device name
> + * @device_name: user space name of device under /dev
This doesn't have actual parameter name.
Btw, I would drop dev_ suffix completely from parameter (you have it
in function name).
> + * @dev_no: pointer to dev_t that this function will populate
Here I'm not sure if dev_ prefix is good to have or not.
> + *
> + * This function converts device names like ttyS0 or ttyUSB1 into dev_t
> + * like (4, 64) or (188, 1). If no corresponding driver is registered then
> + * the function returns -ENODEV.
> + *
> + * Locking: this acquires tty_mutex
...and releases.
Perhaps it makes sense to describe what it protects (like it's done in
some functions around).
> + */
> +int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
const char *name, right?
> +{
> + struct tty_driver *p;
> + int rv, index, prefix_length = 0;
I would keep returned variable on a separate line and name it like
other functions do in this file, i.e. ret.
int ret;
> + while (!isdigit(*(dev_name + prefix_length)) && prefix_length <
> + strlen(dev_name) )
> + prefix_length++;
> +
> + if (prefix_length == strlen(dev_name))
> + return -EINVAL;
Basically, what you need is to get tailing digits, right?
Moreover, there is quite similar piece of code in
tty_find_polling_driver() you may share.
> + mutex_lock(&tty_mutex);
> +
> + list_for_each_entry(p, &tty_drivers, tty_drivers)
> + if (prefix_length == strlen(p->name) && strncmp(dev_name,
> + p->name, prefix_length) == 0) {
> + rv = kstrtoint(dev_name + prefix_length, 10, &index);
> + if (rv) {
> + mutex_unlock(&tty_mutex);
> + return rv;
I would go with goto style in this function (since it has locking involved).
> + }
All together kstrtoint() is invariant here as far as I can see and can
be done out of locking. Also see above comment how to get line index.
> + if (index < p->num) {
> + *dev_no = MKDEV(p->major, p->minor_start + index);
> + mutex_unlock(&tty_mutex);
> + return 0;
> + }
> + }
> +
> + mutex_unlock(&tty_mutex);
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(tty_dev_name_to_number);
--
With Best Regards,
Andy Shevchenko
On Sun, Jun 18, 2017 at 04:27:42PM +0300, Andy Shevchenko wrote:
> This doesn't have actual parameter name.
> Btw, I would drop dev_ suffix completely from parameter (you have it
> in function name).
Good call, thanks.
> > + * Locking: this acquires tty_mutex
>
> ...and releases.
>
> Perhaps it makes sense to describe what it protects (like it's done in
> some functions around).
Yes, will add the description
> > + */
> > +int tty_dev_name_to_number(char *dev_name, dev_t *dev_no)
>
> const char *name, right?
Yes :)
> > + int rv, index, prefix_length = 0;
>
> I would keep returned variable on a separate line and name it like
> other functions do in this file, i.e. ret.
>
> int ret;
Sure
> > + while (!isdigit(*(dev_name + prefix_length)) && prefix_length <
> > + strlen(dev_name) )
> > + prefix_length++;
> > +
> > + if (prefix_length == strlen(dev_name))
> > + return -EINVAL;
>
> Basically, what you need is to get tailing digits, right?
>
> Moreover, there is quite similar piece of code in
> tty_find_polling_driver() you may share.
tty_find_polling_driver does something slightly different. It looks for
digits embedded in a string, so something like kgdboc=ttyS0,115200 where
the digit being sought is 0 after ttyS. So the sanity checks aren't the
same. It also looks for a comma in addition to looking for a number,
which doesn't apply here. Little bit of functionality may be factored
out but that will be too trivial.
While skimming through tty_find_polling_driver, I also noticed that, in
a strict sense, the following check may not be sufficient when name is
prefix of p->name.
if (strncmp(name, p->name, len) != 0)
> > + if (rv) {
>
> > + mutex_unlock(&tty_mutex);
> > + return rv;
>
> I would go with goto style in this function (since it has locking involved).
Sure
>
> > + }
>
> All together kstrtoint() is invariant here as far as I can see and can
> be done out of locking. Also see above comment how to get line index.
Good call, thanks. Think I changed the code afterwards but didn't
notice that kstrtoint no longer needs to be inside the loop and lock.
Best regards,
Okash