From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59920) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3RBW-0001bs-Gt for qemu-devel@nongnu.org; Mon, 30 Nov 2015 11:20:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3RBQ-0006mR-K6 for qemu-devel@nongnu.org; Mon, 30 Nov 2015 11:20:14 -0500 References: <5656892E.9020908@redhat.com> <2BF45FF0-31E6-4901-B4B5-90FF3D672C70@gmail.com> From: Eric Blake Message-ID: <565C7723.5090400@redhat.com> Date: Mon, 30 Nov 2015 09:19:47 -0700 MIME-Version: 1.0 In-Reply-To: <2BF45FF0-31E6-4901-B4B5-90FF3D672C70@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="D6wqXSabFVUoclR71mr7xUEguHBPL8MMM" 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: Programmingkid Cc: Kevin Wolf , qemu-devel qemu-devel , Qemu-block This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --D6wqXSabFVUoclR71mr7xUEguHBPL8MMM Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/27/2015 12:35 PM, Programmingkid wrote: >> 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. 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. 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. >>> + /* 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? 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 >> >>> + /* 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? 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. >=20 > Thank you very much for reviewing my patches. The least you can do for showing that gratitude is to actually improve your next revisions along the lines of the comments you have been given. Quit making it feel like pulling teeth just to get your patches to match the coding conventions prevalent in the project. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --D6wqXSabFVUoclR71mr7xUEguHBPL8MMM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWXHcrAAoJEKeha0olJ0NqZFMH/Ri5tIjZM4XmXbyDslr9n9SN w75oZmoMEymtfqZFjn3S+x7o6SuguWj12vT69pSq73aHF5cxevCvZkQJjPVcmEZY i9fMXxGAx6ozXEVd6ZVyTNNJSRa81mTjZzFbC1pphbKZ4aLU/xOyJMugfR3hnOqx WnBbRA8nwmw5cNOl9gIG5VUQg40ZMqC4w+rEy6ZJnMaYQFmcgyykrHIrSOWKTRrs FYWP4ff5vaUWmERQKaj7Cr7Q/LCDvPyr12dO2Y3WuvRhL4xMiJgiVbN2N2H1O7ti HMkGgmuKWubz/hglgzvpzbc7cxYMf70+xcVZu+rqrmzQjOwqFlQvQnhTs25zXjA= =QKt1 -----END PGP SIGNATURE----- --D6wqXSabFVUoclR71mr7xUEguHBPL8MMM--