From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axwPi-00043L-3k for qemu-devel@nongnu.org; Wed, 04 May 2016 09:00:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axwPV-00037F-U2 for qemu-devel@nongnu.org; Wed, 04 May 2016 09:00:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42298) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axwPV-00030F-Md for qemu-devel@nongnu.org; Wed, 04 May 2016 09:00:13 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AD0CD7F09A for ; Wed, 4 May 2016 13:00:02 +0000 (UTC) From: Markus Armbruster References: <1461903820-3092-1-git-send-email-eblake@redhat.com> <1461903820-3092-11-git-send-email-eblake@redhat.com> <20160503094447.GE2242@work-vm> <87inyv5nv7.fsf@dusky.pond.sub.org> <20160503132358.GH2242@work-vm> <87eg9iyypp.fsf@dusky.pond.sub.org> <20160504092249.GC2302@work-vm> <87lh3qrr4a.fsf@dusky.pond.sub.org> <20160504115616.GE2302@work-vm> Date: Wed, 04 May 2016 15:00:00 +0200 In-Reply-To: <20160504115616.GE2302@work-vm> (David Alan Gilbert's message of "Wed, 4 May 2016 12:56:16 +0100") Message-ID: <87pot2ou5r.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: Amit Shah , famz@redhat.com, qemu-devel@nongnu.org, Juan Quintela "Dr. David Alan Gilbert" writes: > * Markus Armbruster (armbru@redhat.com) wrote: >> "Dr. David Alan Gilbert" writes: >> >> > * Markus Armbruster (armbru@redhat.com) wrote: >> >> "Dr. David Alan Gilbert" writes: >> > >> >> "git-grep assert migration" suggests you do kill the source on certain >> >> programming errors. >> > >> > I'm just trying hard to reduce them; I know I'm not there, but I'd rather >> > we didn't have any - especially on the source side. >> > >> >> I reiterate my point that fancy, untestable error recovery is unlikely >> >> to actually recover. "Fancy" can work, "untestable" might work (but >> >> color me skeptic), but once you got both, you're a dead man walking. >> > >> > Then we should make the error recovery paths easy; at the moment visitor >> > error paths are just too painful. >> >> I've never seen error handling in C that wasn't painful and still >> correct. Surprise me! > > The thing that makes it hard for the visitor code is the need to check > it after every call and the check is complicated. Having to check every call is certainly painful, but there's no general and safe way around it. Accumulating errors that need to be checked only at the end of a job can be less painful, but then the job's code needs to be very carefully written to be safe even in presence of errors. Most code isn't, and some code can't. The check for failure is simple, but annoyingly verbose when the function's return value is useless: Error *err = NULL; foo(..., &err); if (err) { ... } I'm playing with a update to conventions and usage to permit if (!foo(..., &err)) { ... } Just as simple, but more readable. [...] >> I figure we're unlikely to reach consensus on this, so I'd like to >> propose we agree to disagree, and do the following: >> >> * We shelve the de-duplication of JSON formatting (this patch) >> indefinitely. >> >> * We move qjson.c to migration/, next to its only user, and add a >> comment explaining why it migration doesn't want to use general >> infrastructure here (JSON output visitor), but needs its own thing. >> This gets the file covered in MAINTAINERS, and will help prevent it >> growing additional users. >> >> Deal? > > No, sorry; the JSON use in the migration is just a debug thing; > we don't want to maintain a separate JSON instance for it. Well, you already do, except in name. Who else do you think is maintaining qjson.[ch], created by migration people, for migration's use? Certainly not me. If you can't use the general JSON output code I maintain because of special requirements, you get to continue maintaining your own. All 109 SLOC of it. All I'm asking is to make it official, and to deter accidental use of migration's JSON writer instead of the general one.