From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59503) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZofpN-0004H9-3X for qemu-devel@nongnu.org; Tue, 20 Oct 2015 18:56:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZofpI-0001wU-04 for qemu-devel@nongnu.org; Tue, 20 Oct 2015 18:56:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53823) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZofpH-0001wQ-PD for qemu-devel@nongnu.org; Tue, 20 Oct 2015 18:56:15 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 34E6BC0AF78A for ; Tue, 20 Oct 2015 22:56:15 +0000 (UTC) References: <1444968943-11254-1-git-send-email-eblake@redhat.com> <1444968943-11254-5-git-send-email-eblake@redhat.com> <87h9lmat1b.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <5626C68E.2030909@redhat.com> Date: Tue, 20 Oct 2015 16:56:14 -0600 MIME-Version: 1.0 In-Reply-To: <87h9lmat1b.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Ss7nt77owoOENeWKEKh9AGamuIClutexK" Subject: Re: [Qemu-devel] [PATCH v9 04/17] vnc: hoist allocation of VncBasicInfo to callers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Gerd Hoffmann This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ss7nt77owoOENeWKEKh9AGamuIClutexK Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/20/2015 01:38 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> A future qapi patch will rework generated structs with a base >> class to be unboxed. In preparation for that, change the code >> that allocates then populates an info struct to instead merely >> populate the fields of an info field passed in as a parameter. >> Add rudimentary Error handling for cases where the old code >> returned NULL; but as before, callers merely ignore errors for >> now. >=20 > Actually, the call chain rooted at vnc_qmp_event() does handle failure > before this patch. It ignores the error *object* after the patch. >=20 >> @@ -168,42 +169,44 @@ static VncBasicInfo *vnc_basic_info_get(struct s= ockaddr_storage *sa, >> host, sizeof(host), >> serv, sizeof(serv), >> NI_NUMERICHOST | NI_NUMERICSERV)) !=3D 0) = { >> - VNC_DEBUG("Cannot resolve address %d: %s\n", >> - err, gai_strerror(err)); >> - return NULL; >> + error_setg(errp, "Cannot resolve address %d: %s", >> + err, gai_strerror(err)); >=20 > Printing err is fine for a debug message. Less so for an error message= =2E > Drop it? Ah, as in "Cannot resolve address: %s", gai_strerror(err). Sure, sounds okay to me. >> @@ -256,13 +259,10 @@ static const char *vnc_auth_name(VncDisplay *vd)= { >> static VncServerInfo *vnc_server_info_get(VncDisplay *vd) >> { >> VncServerInfo *info; >> - VncBasicInfo *bi =3D vnc_basic_info_get_from_server_addr(vd->lsoc= k); >> - if (!bi) { >> - return NULL; >> - } >> >> info =3D g_malloc(sizeof(*info)); >> - info->base =3D bi; >> + info->base =3D g_malloc(sizeof(*info->base)); >> + vnc_basic_info_get_from_server_addr(vd->lsock, info->base, NULL);= >> info->has_auth =3D true; >> info->auth =3D g_strdup(vnc_auth_name(vd)); >> return info; >=20 > Uh, doesn't this change the return value when getsockname() in > vnc_basic_info_get_from_server_addr() fails? Hmm. I wrote the patch back in July (wow - review has been taking a while...), don't know what I was thinking. Yes, I need to fix this to return NULL in the same situations the pre-patch version did, or else pass errp to the caller (looks like just one: vnc_qmp_event()). Or maybe I was intentionally thinking that a best-effort result was appropriate, particularly since the next patch gets rid of the base member and therefore the possibility of info->base being NULL (maybe that just means I rebased my series wrong when splitting one patch into two). Will fix. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Ss7nt77owoOENeWKEKh9AGamuIClutexK 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/ iQEcBAEBCAAGBQJWJsaOAAoJEKeha0olJ0NqLV8IAIhsG0BDoDzQ1wlWl+hNziVu 7hvd2MImnOTMqYwmi3R5xRLFReW6oJ7Bw9oQTIKiyHds+5tCGN90VK/yjIiI/MUK Anvn89SRbWM3RHU6aF8MPf0IWQsOX9HdOM4hcheoXouVRF3AQPPsBT260E9QtJqR xbL/aGFr0u4P+qeKMgflIkRfZ/DmV+OZbqhrY8Itv2ic+5pq08EXNHHzRtUFvhsT wsZWOMSVPpxvIFrCpMXrk5snRAI5WAOb/U0DvddupEir0wWxPavQ/xoDx/Rwc1Gc n9c4utHbVJCTOrBFyyC523H5BODOGGiYcLORr1l8Q+c1ilzO8ctdTwTv2jo8zOs= =+57Z -----END PGP SIGNATURE----- --Ss7nt77owoOENeWKEKh9AGamuIClutexK--