* 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:
> >>
> >> > * 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.
Yes; output visitors would seem to be the easiest case though?
> 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)) {
> ...
> }
If that became;
if (!foo(..., &err) ||
!foo(..., &err) ||
!foo(..., &err)) {
...
}
That would be both readable and not verbose.
> 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.
That came from migration? Really? I didn't think we used JSON at
all until last year.
> 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.
Yeh; I'd love to share the JSON code; just lets try and avoid anything that
can kill the source, however broken the migration.
Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
