From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35165) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3RHs-0002gw-SY for qemu-devel@nongnu.org; Mon, 30 Nov 2015 11:26:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3RHs-0000vE-1A for qemu-devel@nongnu.org; Mon, 30 Nov 2015 11:26:48 -0500 Date: Mon, 30 Nov 2015 17:26:39 +0100 From: Kevin Wolf Message-ID: <20151130162639.GI4494@noname.str.redhat.com> References: <5656892E.9020908@redhat.com> <2BF45FF0-31E6-4901-B4B5-90FF3D672C70@gmail.com> <565C7723.5090400@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="IDYEmSnFhs3mNXr+" Content-Disposition: inline In-Reply-To: <565C7723.5090400@redhat.com> Subject: Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Programmingkid , qemu-devel qemu-devel , Qemu-block --IDYEmSnFhs3mNXr+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 30.11.2015 um 17:19 hat Eric Blake geschrieben: > On 11/27/2015 12:35 PM, Programmingkid wrote: >=20 > >> Unusual indentation; more typical is: > >> > >> | static kern_return_t FindEjectableOpticalMedia(io_iterator_t > >> *mediaIterator, > >> | char *mediatType) > >=20 > > I agree. I wanted the second long to be right justified with the 80 cha= racter line count. >=20 > No. We don't right-justify code to 80 columns. That's not how it is > done. Trying to do it just makes you look like the proverbial 'kid' in > your pseudonym, rather than an adult to be taken seriously. >=20 > Really, PLEASE follow the indentation patterns of the rest of the code > base - where continued lines are left-justified to be underneath the > character after (, and NOT right-justified to 80 columns. Violating > style doesn't make your code invalid, but does make your patches less > likely to be applied. >=20 >=20 > >>> + /* If you found a match, leave the loop */ > >>> + if (*mediaIterator !=3D 0) { > >>> + DPRINTF("Matching using %s\n", matching_array[index]); > >>> + snprintf(mediaType, strlen(matching_array[index])+1, "%s= ", > >> > >> Spaces around binary '+'. > >=20 > > What's wrong with no spaces around the plus sign? >=20 > Again, the prevailing conventions in the code base is that you put > spaces around every binary operator. Yes, there is existing old code > that does not meet the conventions, but it is not an excuse to add new > code that is gratuitously different. >=20 > >=20 > >> > >>> + /* if a working partition on the device was not found */ > >>> + if (partition_found =3D=3D false) { > >>> + error_setg(errp, "Error: Failed to find a working partition = on " > >>> + = "disc!\n"); > >> > >> and I already pointed out on v8 that this is not the correct usage of > >> error_setg(). So, here's hoping v10 addresses the comments here and > >> elsewhere. > >=20 > > Kevin Wolf wanted it this way. What would you do instead? >=20 > Keven and I both want you to use error_setg(), but to use it correctly - > and the correct way is to NOT supply a trailing \n. Nor leading "Error:", for that matter. Kevin --IDYEmSnFhs3mNXr+ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJWXHi/AAoJEH8JsnLIjy/WN9IP/2cQSlJXGl/SLtOtmq3qMbRZ BAjQrEngF0Owy5ZmIq3dhmnTo5CvdZf4Wq3E7kjJRGNOi0U+Bw0Y/4KbRJH8/SQ7 v2eCnojmIebeJHPfMLApyxs7Oy+GZtu/nymr9qeqMFpqLDw3Hugs7LYUjMpCblxZ dPCnADt0Us37N3JMWx9pX5pFENwSDIQ9vd1iBb6sUb9RaIPizz4E1GNCuMaDaGB8 XiwIUnnnVC0S2wT8cCRVefFHkgIjcJH7/ls15t63lgtQ0d4/kT6jABUPbJEHP6os rz6CBCje7ME/FwjqnON3lBErsav5hcVImmR1oLb636hUwtSKlxfZQw4Bwe8eOOGs RnRvl96JmwHRSoqzfZgafPRITPcpYXao9yjfR0ufTEjKtBYQQyMYljrd6u48Tu9l ip1Y0LdZiwEeIKHVGYnLtJHPfSNdauukkwh5omEJiE8peJaquYcvH7jysobHxUCe gWxndVN7AYQA7XCBAceoCG3d6Xv4L1z8ogbPfk0B3Tm5nO1x9DmdF4W+Xipn6grJ bG67ODkxG5UaDOEPUI8yO0JbltuELELbi2XAO0g57ApszRSani6hMC4HHQuEbTay aDek+fbasXugDOVQeDTuLZzM+UTPkf+CEd+xznnzq9xm17gbYN/iJGB56P1TzwYF uSgdJhVIGrN2dDrIJaxE =O+Ae -----END PGP SIGNATURE----- --IDYEmSnFhs3mNXr+--