Page MenuHomeFreeBSD
Differential D28218 Diff 82653 sys/dev/evdev/cdev.c

Changeset View

Standalone View

View Options

sys/dev/evdev/cdev.c

Show First 20 Lines • Show All 72 Lines • ▼ Show 20 Lines
static d_write_t evdev_write; static d_write_t evdev_write;
static d_ioctl_t evdev_ioctl; static d_ioctl_t evdev_ioctl;
static d_poll_t evdev_poll; static d_poll_t evdev_poll;
static d_kqfilter_t evdev_kqfilter; static d_kqfilter_t evdev_kqfilter;
static int evdev_kqread(struct knote *kn, long hint); static int evdev_kqread(struct knote *kn, long hint);
static void evdev_kqdetach(struct knote *kn); static void evdev_kqdetach(struct knote *kn);
static void evdev_dtor(void *); static void evdev_dtor(void *);
static int evdev_ioctl_eviocgbit(struct evdev_dev *, int, int, caddr_t); static int evdev_ioctl_eviocgbit(struct evdev_dev *, int, int, caddr_t,
struct thread *);
static void evdev_client_filter_queue(struct evdev_client *, uint16_t); static void evdev_client_filter_queue(struct evdev_client *, uint16_t);
static struct cdevsw evdev_cdevsw = { static struct cdevsw evdev_cdevsw = {
.d_version = D_VERSION, .d_version = D_VERSION,
.d_open = evdev_open, .d_open = evdev_open,
.d_read = evdev_read, .d_read = evdev_read,
.d_write = evdev_write, .d_write = evdev_write,
.d_ioctl = evdev_ioctl, .d_ioctl = evdev_ioctl,
▲ Show 20 Lines • Show All 481 Lines • ▼ Show 20 Lines case EVIOCSCLOCKID:
default: default:
return (EINVAL); return (EINVAL);
} }
} }
/* evdev variable-length ioctls handling */ /* evdev variable-length ioctls handling */
switch (IOCBASECMD(cmd)) { switch (IOCBASECMD(cmd)) {
case EVIOCGNAME(0): case EVIOCGNAME(0):
strlcpy(data, evdev->ev_name, len); /* Linux evdev does not terminate truncated strings with 0 */
limit = MIN(strlen(evdev->ev_name) + 1, len);
memcpy(data, evdev->ev_name, limit);
td->td_retval[0] = limit;
wulfUnsubmitted
Done
Inline Actions

Could you use limit variable to be consistent with existing code?

Also couple words about why strlen/memcpy is used instead of strlcopy would be nice.

wulf: Could you use limit variable to be consistent with existing code? Also couple words about why…
ankohuu_outlook.comAuthorUnsubmitted
Not Done
Inline Actions

Could you use limit variable to be consistent with existing code?

ok

Also couple words about why strlen/memcpy is used instead of strlcopy would be nice.

I missed the reason for this modification, on Linux when ioctl is EVIOCGNAME | EVIOCGPHYS | EVIOCGUNIQ, it use str_to_user to copy data.
str_to_user has a different behavior as strlcpy in FreeBSD kernel, when args->len is equal or smaller than the length of source string,
str_to_user copy "len" bytes to dst and strlcpy copy "len - 1" to dst and the last one is zero

static int str_to_user(const char *str, unsigned int maxlen, void __user *p)         
{                                                                                
    int len;                                                                     
                                                                                 
    if (!str)                                                                    
        return -ENOENT;                                                          
                                                                                 
    len = strlen(str) + 1;                                                       
    if (len > maxlen)                                                            
        len = maxlen;                                                            
                                                                                 
    return copy_to_user(p, str, len) ? -EFAULT : len;                            
}
ankohuu_outlook.com: > Could you use limit variable to be consistent with existing code? ok > > Also couple words…
wulfUnsubmitted
Not Done
Inline Actions

This requires one line comment as it is not obvious and refers Linux sources.

wulf: This requires one line comment as it is not obvious and refers Linux sources.
return (0); return (0);
case EVIOCGPHYS(0): case EVIOCGPHYS(0):
if (evdev->ev_shortname[0] == 0) if (evdev->ev_shortname[0] == 0)
return (ENOENT); return (ENOENT);
strlcpy(data, evdev->ev_shortname, len); limit = MIN(strlen(evdev->ev_shortname) + 1, len);
memcpy(data, evdev->ev_shortname, limit);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGUNIQ(0): case EVIOCGUNIQ(0):
if (evdev->ev_serial[0] == 0) if (evdev->ev_serial[0] == 0)
return (ENOENT); return (ENOENT);
strlcpy(data, evdev->ev_serial, len); limit = MIN(strlen(evdev->ev_serial) + 1, len);
memcpy(data, evdev->ev_serial, limit);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGPROP(0): case EVIOCGPROP(0):
limit = MIN(len, bitstr_size(INPUT_PROP_CNT)); limit = MIN(len, bitstr_size(INPUT_PROP_CNT));
memcpy(data, evdev->ev_prop_flags, limit); memcpy(data, evdev->ev_prop_flags, limit);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGMTSLOTS(0): case EVIOCGMTSLOTS(0):
/* EVIOCGMTSLOTS always returns 0 on success */
if (evdev->ev_mt == NULL) if (evdev->ev_mt == NULL)
return (EINVAL); return (EINVAL);
if (len < sizeof(uint32_t)) if (len < sizeof(uint32_t))
return (EINVAL); return (EINVAL);
code = *(uint32_t *)data; code = *(uint32_t *)data;
if (!ABS_IS_MT(code)) if (!ABS_IS_MT(code))
return (EINVAL); return (EINVAL);
nvalues = nvalues =
MIN(len / sizeof(int32_t) - 1, MAXIMAL_MT_SLOT(evdev) + 1); MIN(len / sizeof(int32_t) - 1, MAXIMAL_MT_SLOT(evdev) + 1);
for (int i = 0; i < nvalues; i++) for (int i = 0; i < nvalues; i++)
((int32_t *)data)[i + 1] = ((int32_t *)data)[i + 1] =
evdev_get_mt_value(evdev, i, code); evdev_get_mt_value(evdev, i, code);
wulfUnsubmitted
Not Done
Inline Actions

Do EVIOCGMTSLOTS returns the length?

wulf: Do EVIOCGMTSLOTS returns the length?
ankohuu_outlook.comAuthorUnsubmitted
Not Done
Inline Actions

Do EVIOCGMTSLOTS returns the length?

It returns 0 when correct,
when device is not MT or ev_code passed is wrong it returns EINVAL,
when something wrong happened in copy, it returns EFAULT.

ankohuu_outlook.com: > Do EVIOCGMTSLOTS returns the length? It returns 0 when correct, when device is not MT or…
wulfUnsubmitted
Not Done
Inline Actions

This requires one line comment to as it does not matches other cases.

wulf: This requires one line comment to as it does not matches other cases.
return (0); return (0);
case EVIOCGKEY(0): case EVIOCGKEY(0):
limit = MIN(len, bitstr_size(KEY_CNT)); limit = MIN(len, bitstr_size(KEY_CNT));
EVDEV_LOCK(evdev); EVDEV_LOCK(evdev);
evdev_client_filter_queue(client, EV_KEY); evdev_client_filter_queue(client, EV_KEY);
memcpy(data, evdev->ev_key_states, limit); memcpy(data, evdev->ev_key_states, limit);
EVDEV_UNLOCK(evdev); EVDEV_UNLOCK(evdev);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGLED(0): case EVIOCGLED(0):
limit = MIN(len, bitstr_size(LED_CNT)); limit = MIN(len, bitstr_size(LED_CNT));
EVDEV_LOCK(evdev); EVDEV_LOCK(evdev);
evdev_client_filter_queue(client, EV_LED); evdev_client_filter_queue(client, EV_LED);
memcpy(data, evdev->ev_led_states, limit); memcpy(data, evdev->ev_led_states, limit);
EVDEV_UNLOCK(evdev); EVDEV_UNLOCK(evdev);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGSND(0): case EVIOCGSND(0):
limit = MIN(len, bitstr_size(SND_CNT)); limit = MIN(len, bitstr_size(SND_CNT));
EVDEV_LOCK(evdev); EVDEV_LOCK(evdev);
evdev_client_filter_queue(client, EV_SND); evdev_client_filter_queue(client, EV_SND);
memcpy(data, evdev->ev_snd_states, limit); memcpy(data, evdev->ev_snd_states, limit);
EVDEV_UNLOCK(evdev); EVDEV_UNLOCK(evdev);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGSW(0): case EVIOCGSW(0):
limit = MIN(len, bitstr_size(SW_CNT)); limit = MIN(len, bitstr_size(SW_CNT));
EVDEV_LOCK(evdev); EVDEV_LOCK(evdev);
evdev_client_filter_queue(client, EV_SW); evdev_client_filter_queue(client, EV_SW);
memcpy(data, evdev->ev_sw_states, limit); memcpy(data, evdev->ev_sw_states, limit);
EVDEV_UNLOCK(evdev); EVDEV_UNLOCK(evdev);
td->td_retval[0] = limit;
return (0); return (0);
case EVIOCGBIT(0, 0) ... EVIOCGBIT(EV_MAX, 0): case EVIOCGBIT(0, 0) ... EVIOCGBIT(EV_MAX, 0):
type_num = IOCBASECMD(cmd) - EVIOCGBIT(0, 0); type_num = IOCBASECMD(cmd) - EVIOCGBIT(0, 0);
debugf(client, "EVIOCGBIT(%d): data=%p, len=%d", type_num, debugf(client, "EVIOCGBIT(%d): data=%p, len=%d", type_num,
data, len); data, len);
return (evdev_ioctl_eviocgbit(evdev, type_num, len, data)); return (evdev_ioctl_eviocgbit(evdev, type_num, len, data, td));
wulfUnsubmitted
Not Done
Inline Actions

IMO returning of length in len or limit field rather in td->td_retval would make the code cleaner.

wulf: IMO returning of length in len or limit field rather in td->td_retval would make the code…
ankohuu_outlook.comAuthorUnsubmitted
Done
Inline Actions

IMO returning of length in len or limit field rather in td->td_retval would make the code cleaner.

I didn't understand, do you mean I need change return value's meaning of evdev_ioctl_eviocgbit, now it represents operation EVIOCGBIT's error code,
should I mix length meaning in it for return actual length to funcion evdev_do_ioctl?
Or should I change evdev_ioctl_eviocgbit's argument len to &len, in order to return actual length to evdev_do_ioctl?
I think this is a syscall and when no error accurs I need a positive return, I have to use td->td_retval[0] to store it.

ankohuu_outlook.com: > IMO returning of length in len or limit field rather in td->td_retval would make the code…
wulfUnsubmitted
Not Done
Inline Actions

Or should I change evdev_ioctl_eviocgbit's argument len to &len

I meant exactly that. Passing &len as output parameter looks cleaner than passing td as output parameter. And does not require 1 extra argument to evdev_ioctl_eviocgbit().

wulf: > Or should I change evdev_ioctl_eviocgbit's argument len to &len I meant exactly that.
} }
return (EINVAL); return (EINVAL);
} }
static int static int
evdev_ioctl_eviocgbit(struct evdev_dev *evdev, int type, int len, caddr_t data) evdev_ioctl_eviocgbit(struct evdev_dev *evdev, int type, int len, caddr_t data,
struct thread *td)
{ {
unsigned long *bitmap; unsigned long *bitmap;
int limit; int limit;
switch (type) { switch (type) {
case 0: case 0:
bitmap = evdev->ev_type_flags; bitmap = evdev->ev_type_flags;
limit = EV_CNT; limit = EV_CNT;
Show All 27 Lines case EV_SW:
limit = SW_CNT; limit = SW_CNT;
break; break;
case EV_FF: case EV_FF:
/* /*
* We don't support EV_FF now, so let's * We don't support EV_FF now, so let's
* just fake it returning only zeros. * just fake it returning only zeros.
*/ */
bzero(data, len); bzero(data, len);
td->td_retval[0] = len;
return (0); return (0);
default: default:
return (ENOTTY); return (ENOTTY);
} }
/* /*
* Clear ioctl data buffer in case it's bigger than * Clear ioctl data buffer in case it's bigger than
* bitmap size * bitmap size
*/ */
bzero(data, len); bzero(data, len);
limit = bitstr_size(limit); limit = bitstr_size(limit);
len = MIN(limit, len); len = MIN(limit, len);
memcpy(data, bitmap, len); memcpy(data, bitmap, len);
td->td_retval[0] = len;
return (0); return (0);
} }
void void
evdev_revoke_client(struct evdev_client *client) evdev_revoke_client(struct evdev_client *client)
{ {
EVDEV_LIST_LOCK_ASSERT(client->ec_evdev); EVDEV_LIST_LOCK_ASSERT(client->ec_evdev);
▲ Show 20 Lines • Show All 211 Lines • Show Last 20 Lines