From patchwork Sun Jun 25 18:06:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Torokhov X-Patchwork-Id: 9808293 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5C1B060329 for ; Sun, 25 Jun 2017 18:06:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3376B283BB for ; Sun, 25 Jun 2017 18:06:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 24F582858B; Sun, 25 Jun 2017 18:06:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5394C283BB for ; Sun, 25 Jun 2017 18:06:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751335AbdFYSGO (ORCPT ); Sun, 25 Jun 2017 14:06:14 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:36442 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751244AbdFYSGN (ORCPT ); Sun, 25 Jun 2017 14:06:13 -0400 Received: by mail-pf0-f193.google.com with SMTP id z6so6442023pfk.3; Sun, 25 Jun 2017 11:06:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=i1XzE+NZNVOwOfMZB1hxLRPPdFDwKyx+U5h/0XKBWV8=; b=Xolj7AO+RRlmU/SLR/d8aB2ayS9gVjf3jOWyw7MD3b1cKLBUawOjy9WIWeeJHY6wAm xXJ4ohd5J/QS2x8EwSLIfXa2Z1noG98WHG8l4tK2ND3jvwKAJpLGIIv0YyL1mQNGLSqH 0iDskELAjecwndgf6korCRwvnV/DjjA1zoJDxhONOaVamT6+IXPPLildFfmoy+cNfFvF 4WuUzhoDievyRvLWct1n/8+7tySFWXjCLe6joqhDC0lDYkRfvfzXTfeKtTytNH2bu0NP d/OTuhnBlQEhf5xHuwmTFn82JEOFVUx7YcR8zWZ3Op3SHL7nhviMz3qSsK87bvqhEQvW N7bA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=i1XzE+NZNVOwOfMZB1hxLRPPdFDwKyx+U5h/0XKBWV8=; b=ODR0zOD8sULRS66SCcczUlRUlZYZrOn5HsCBi1LBO9dv1G7sNSmsKHzK/Dg0CJHoQP r84XvQWJgk6ck6uwiDUqzyUo9rHwO+SAQSZwMoQJKUs/v8va96e5DbNJwZG+HVKzzF3/ meGd3JkilDm2563btPET8WqHXU7llvPgnTUICAhmvRxn6Tzw3zPB38sGlINOh2+mrtsm ldFFSxSeddWS5rn1KyE8z6rjA+OTXYFbdQvnGFu3GkYCxfs9PFXu1fr6D9seojUT97/G xhmhdEfrv+FJPaObtcMtUqOmPY7Dg20Fd0V7eSNw/9GftyLxJSNP/r2EuGIfI088iFKb vbLg== X-Gm-Message-State: AKS2vOwzZPleU3eyY+EtEXw4xsnoe22oJ9PT8r/ZFWKm4giBs4aZe7jf efOhsMX4qqpqlw== X-Received: by 10.98.86.132 with SMTP id h4mr17715267pfj.205.1498413972512; Sun, 25 Jun 2017 11:06:12 -0700 (PDT) Received: from dtor-ws ([2620:0:1000:1311:7575:c536:f4cc:aa58]) by smtp.gmail.com with ESMTPSA id g79sm21921508pfg.121.2017.06.25.11.06.11 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Sun, 25 Jun 2017 11:06:11 -0700 (PDT) Date: Sun, 25 Jun 2017 11:06:09 -0700 From: Dmitry Torokhov To: Nick Desaulniers Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4] Input: mousedev - fix implicit conversion warning Message-ID: <20170625180609.GA8697@dtor-ws> References: <20170526154029.9405-1-nick.desaulniers@gmail.com> <20170530054151.16639-1-nick.desaulniers@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170530054151.16639-1-nick.desaulniers@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Nick, On Mon, May 29, 2017 at 10:41:51PM -0700, Nick Desaulniers wrote: > Clang warns: > > drivers/input/mousedev.c:653:63: error: implicit conversion from 'int' > to 'signed char' changes value from 200 to -56 > [-Wconstant-conversion] > client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200; > ~ ^~~ > > As far as I can tell, from > > http://www.computer-engineering.org/ps2mouse/ > > Under "Command Set" > "0xE9 (Status Request)" > > the value 200 is a valid sample rate. Using unsigned char [], rather than > signed char [], for client->ps2 silences this warning. > > Also moves some reused logic into a helper function. > > Signed-off-by: Nick Desaulniers > --- > What's new in v4: > * replace mousedev_limit_delta() with update_clamped() that also updates > the ps2_data and delta values. The use of the temporary val should > avoid integral conversion and promotion issues. > > drivers/input/mousedev.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) > > diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c > index 0e0ff84088fd..e645b8c6f2cb 100644 > --- a/drivers/input/mousedev.c > +++ b/drivers/input/mousedev.c > @@ -103,7 +103,7 @@ struct mousedev_client { > spinlock_t packet_lock; > int pos_x, pos_y; > > - signed char ps2[6]; > + unsigned char ps2[6]; > unsigned char ready, buffer, bufsiz; > unsigned char imexseq, impsseq; > enum mousedev_emul mode; > @@ -571,27 +571,27 @@ static int mousedev_open(struct inode *inode, struct file *file) > return error; > } > > -static inline int mousedev_limit_delta(int delta, int limit) > +static inline void > +update_clamped(unsigned char *ps2_data, int *delta, int limit) > { > - return delta > limit ? limit : (delta < -limit ? -limit : delta); > + int val = *delta > limit ? limit : (*delta < -limit ? -limit : *delta); > + *ps2_data = (unsigned char) val; > + *delta -= val; Since the time the code was written we got nice helpers to clamp the values. Does the following work for you or it still leaves clang unhappy? Thanks. diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0e0ff84088fd..6ef0c5314f69 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -15,6 +15,7 @@ #define MOUSEDEV_MINORS 31 #define MOUSEDEV_MIX 63 +#include #include #include #include @@ -103,7 +104,7 @@ struct mousedev_client { spinlock_t packet_lock; int pos_x, pos_y; - signed char ps2[6]; + u8 ps2[6]; unsigned char ready, buffer, bufsiz; unsigned char imexseq, impsseq; enum mousedev_emul mode; @@ -291,11 +292,10 @@ static void mousedev_notify_readers(struct mousedev *mousedev, } client->pos_x += packet->dx; - client->pos_x = client->pos_x < 0 ? - 0 : (client->pos_x >= xres ? xres : client->pos_x); + client->pos_x = clamp_val(client->pos_x, 0, xres); + client->pos_y += packet->dy; - client->pos_y = client->pos_y < 0 ? - 0 : (client->pos_y >= yres ? yres : client->pos_y); + client->pos_y = clamp_val(client->pos_y, 0, yres); p->dx += packet->dx; p->dy += packet->dy; @@ -571,44 +571,48 @@ static int mousedev_open(struct inode *inode, struct file *file) return error; } -static inline int mousedev_limit_delta(int delta, int limit) -{ - return delta > limit ? limit : (delta < -limit ? -limit : delta); -} - -static void mousedev_packet(struct mousedev_client *client, - signed char *ps2_data) +static void mousedev_packet(struct mousedev_client *client, u8 *ps2_data) { struct mousedev_motion *p = &client->packets[client->tail]; + s8 dx, dy, dz; + + dx = clamp_val(p->dx, -127, 127); + p->dx -= dx; + + dy = clamp_val(p->dy, -127, 127); + p->dy -= dy; - ps2_data[0] = 0x08 | - ((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07); - ps2_data[1] = mousedev_limit_delta(p->dx, 127); - ps2_data[2] = mousedev_limit_delta(p->dy, 127); - p->dx -= ps2_data[1]; - p->dy -= ps2_data[2]; + ps2_data[0] = BIT(3); + ps2_data[0] |= ((dx & BIT(7)) >> 3) | ((dy & BIT(7)) >> 2); + ps2_data[0] |= p->buttons & 0x07; switch (client->mode) { case MOUSEDEV_EMUL_EXPS: - ps2_data[3] = mousedev_limit_delta(p->dz, 7); - p->dz -= ps2_data[3]; - ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1); + dz = clamp_val(p->dz, -7, 7); + p->dz -= dz; + + ps2_data[3] = (dz & 0x0f) | ((p->buttons & 0x18) << 1); client->bufsiz = 4; break; case MOUSEDEV_EMUL_IMPS: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); - ps2_data[3] = mousedev_limit_delta(p->dz, 127); - p->dz -= ps2_data[3]; + dz = clamp_val(p->dz, -127, 127); + p->dz -= dz; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + ps2_data[3] = dz; + client->bufsiz = 4; break; case MOUSEDEV_EMUL_PS2: default: - ps2_data[0] |= - ((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1); p->dz = 0; + + ps2_data[0] |= ((p->buttons & 0x10) >> 3) | + ((p->buttons & 0x08) >> 1); + client->bufsiz = 3; break; } @@ -714,7 +718,7 @@ static ssize_t mousedev_read(struct file *file, char __user *buffer, { struct mousedev_client *client = file->private_data; struct mousedev *mousedev = client->mousedev; - signed char data[sizeof(client->ps2)]; + u8 data[sizeof(client->ps2)]; int retval = 0; if (!client->ready && !client->buffer && mousedev->exist &&