From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Dmitry V. Levin" Subject: Re: [PATCH v4 4/5] drm: Add decoding of i915 ioctls Date: Fri, 11 Sep 2015 15:04:14 +0300 Message-ID: <20150911120414.GA6177@altlinux.org> References: <1440420170-13337-1-git-send-email-patrik.jakobsson@linux.intel.com> <1440420170-13337-5-git-send-email-patrik.jakobsson@linux.intel.com> <20150908011811.GB17781@altlinux.org> <20150911113102.GB28550@patrik-desktop.isw.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0710701930==" Return-path: Received: from pegasus3.altlinux.org (pegasus3.altlinux.org [194.107.17.103]) by gabe.freedesktop.org (Postfix) with ESMTP id 3B98F6E3C3 for ; Fri, 11 Sep 2015 05:04:17 -0700 (PDT) In-Reply-To: <20150911113102.GB28550@patrik-desktop.isw.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: strace-devel@lists.sourceforge.net Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0710701930== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="YZ5djTAD1cGYuMQK" Content-Disposition: inline --YZ5djTAD1cGYuMQK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 11, 2015 at 01:31:02PM +0200, Patrik Jakobsson wrote: > On Tue, Sep 08, 2015 at 04:18:11AM +0300, Dmitry V. Levin wrote: > > On Mon, Aug 24, 2015 at 02:42:49PM +0200, Patrik Jakobsson wrote: > > > +static int i915_getparam(struct tcb *tcp, const unsigned int code, l= ong arg) > > > +{ > > > + struct drm_i915_getparam param; > > > + int value; > > > + > > > + if (umove(tcp, arg, ¶m)) > > > + return RVAL_DECODED; > > > + > > > + if (entering(tcp)) { > > > + tprints(", {param=3D"); > > > + printxval(drm_i915_getparams, param.param, "I915_PARAM_???"); > > > + } else if (exiting(tcp)) { > > > + if (umove(tcp, (long)param.value, &value)) > > > + return RVAL_DECODED; > >=20 > > Since part of param has already been printed, RVAL_DECODED shouldn't be > > returned here. For the same reason, RVAL_DECODED shouldn't be returned > > earlier in this function. >=20 > The usage of RVAL_DECODED is quite confusing. RVAL_DECODED to me should b= e "We > decoded this don't do any fallback". How did you intent it to be used? RVAL_DECODED itself is trivial: by setting this flag parser tells its caller that decoding is finished on entering and it shouldn't be called on exiting of this syscall. Setting this flag on exiting has no effect. With regards to ioctl parsers, there might be some confusion because they also have this unusual return code +1 semantics. In this example, the problem with "return RVAL_DECODED" statements is that if they happen on exiting, it means that something has already been printed on entering already and by returning RVAL_DECODED parser tells its caller to perform the default action that also prints something. That is, ioctl parser should return - on entering: + any value (it's ignored) without RVAL_DECODED flag set: continue processing on exiting; + RVAL_DECODED: skip processing on exiting and tell the caller to perform default processing on entering; + RVAL_DECODED | (1 + RVAL_*): skip processing on exiting and tell the caller to skip default processing; - on exiting: + 0 (with or without RVAL_DECODED set, it's ignored anyway): tell the caller to perform default processing on exiting; + 1 + RVAL_* (RVAL_DECODED is ignored): tell the caller to skip default processing. > > > + break; > > > + default: > > > + tprintf("%d", value); > >=20 > > Likewise. > >=20 > > > + } > > > + tprints("}"); > > > + } > > > + > > > + return RVAL_DECODED | 1; > >=20 > > This shouldn't be returned on entering(tcp). >=20 > If all decoding would be done when entering is finished, wouldn't it make= sense > to allow RVAL_DECODED here? Still don't understand how this is intended t= o work. Usally only IOW parsers return codes with RVAL_DECODED set on entering. IOWR also print something on exiting so they shouldn't normally return RVAL_DECODED on entering. --=20 ldv --YZ5djTAD1cGYuMQK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlXywz4ACgkQfKvmrJ41Nh7bRgCgmW8a6FWuUr5hlo5wbyi44GP3 9bsAn0fIE9dFBiWCcf217F5vHBRXgdyh =aF3F -----END PGP SIGNATURE----- --YZ5djTAD1cGYuMQK-- --===============0710701930== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0710701930==--