"Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: > * Markus Armbruster (arm...@redhat.com) wrote: >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: >> >> > * Markus Armbruster (arm...@redhat.com) wrote: >> >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> 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.