Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751837AbdIVByl (ORCPT ); Thu, 21 Sep 2017 21:54:41 -0400 Received: from mail-io0-f180.google.com ([209.85.223.180]:51731 "EHLO mail-io0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751704AbdIVByk (ORCPT ); Thu, 21 Sep 2017 21:54:40 -0400 X-Google-Smtp-Source: AOwi7QCV9wEnD9bXk+G4yLm0j9d8UzLdtFIS3beeJikWIqLjzy4Z6lRfsmkZWPrd6a6NIVZrovg0ltONq3n+ygax4iU= MIME-Version: 1.0 In-Reply-To: References: From: Baolin Wang Date: Fri, 22 Sep 2017 09:54:39 +0800 Message-ID: Subject: Re: [RFC PATCH 4/7] sound: core: Avoid using timespec for struct snd_rawmidi_status To: Arnd Bergmann Cc: Jaroslav Kysela , Takashi Iwai , Liam Girdwood , Ingo Molnar , Takashi Sakamoto , SF Markus Elfring , Dan Carpenter , jeeja.kp@intel.com, Vinod Koul , dharageswari.r@intel.com, guneshwor.o.singh@intel.com, Bhumika Goyal , gudishax.kranthikumar@intel.com, Naveen M , hardik.t.shah@intel.com, Arvind Yadav , Fabian Frederick , Mark Brown , Deepa Dinamani , alsa-devel@alsa-project.org, Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2862 Lines: 69 Hi Arnd, On 21 September 2017 at 20:56, Arnd Bergmann wrote: > On Thu, Sep 21, 2017 at 8:18 AM, Baolin Wang wrote: > >> - case SNDRV_RAWMIDI_IOCTL_STATUS: >> +#if __BITS_PER_LONG == 32 >> + case SNDRV_RAWMIDI_IOCTL_STATUS32: >> + { >> + int err = 0; >> + struct snd_rawmidi_status32 __user *status = argp; >> + struct snd_rawmidi_status32 status32; >> + struct snd_rawmidi_status64 status64; >> + >> + if (copy_from_user(&status32, argp, >> + sizeof(struct snd_rawmidi_status32))) >> + return -EFAULT; >> + switch (status32.stream) { >> + case SNDRV_RAWMIDI_STREAM_OUTPUT: >> + if (rfile->output == NULL) >> + return -EINVAL; >> + err = snd_rawmidi_output_status(rfile->output, &status64); >> + break; >> + case SNDRV_RAWMIDI_STREAM_INPUT: >> + if (rfile->input == NULL) >> + return -EINVAL; >> + err = snd_rawmidi_input_status(rfile->input, &status64); >> + break; >> + default: >> + return -EINVAL; >> + } >> + if (err < 0) >> + return err; >> + >> + if (put_user(status64.stream, &status->stream) || >> + put_user(status64.tstamp.tv_sec, &status->tstamp.tv_sec) || >> + put_user(status64.tstamp.tv_nsec, &status->tstamp.tv_nsec) || >> + put_user(status64.avail, &status->avail) || >> + put_user(status64.xruns, &status->xruns)) >> + return -EFAULT; >> + return 0; >> + } > > This follows the existing coding style for the other functions, but I think > it would be nicer to express the last part as > > status32 = (struct snd_rawmidi_status32) { > .stream = status->stream, > .tstamp.tv_sec, &status->tstamp.tv_sec, > .tstamp.tv_nsec, &status->tstamp.tv_nsec, > .avail, &status->avail, > .xruns, &status->xruns, > }; > if (copy_to_user(status, &status32, sizeof(*status)) > return -EFAULT; > return 0; > > It's completely equivalent, I just find my version easier to read, and > it should produce slightly better object code. > > Maybe the maintainers have a preference, or there might be > a good reason to use the series of put_user() instead. I just saw there are not many put_user() will be used in this function, but I agree with you and I like to change as you suggested. -- Baolin.wang Best Regards