19.09.2019 12:17, Kevin Wolf wrote: > Am 18.09.2019 um 19:10 hat Eric Blake geschrieben: >> On 9/18/19 8:02 AM, Vladimir Sementsov-Ogievskiy wrote: >>> + */ >>> +#define MAKE_ERRP_SAFE(errp) \ >>> +g_auto(ErrorPropagationStruct) (__auto_errp_prop) = {.errp = (errp)}; \ >>> +if ((errp) == NULL || *(errp) == error_abort || *(errp) == error_fatal) { \ >>> + (errp) = &__auto_errp_prop.local_err; \ >>> +} >> >> Not written to take a trailing semicolon in the caller. >> >> You could even set __auto_errp_prop unconditionally rather than trying >> to reuse incoming errp (the difference being that error_propagate() gets >> called more frequently). > > I think this difference is actually a problem. > > When debugging things, I hate error_propagate(). It means that the Error > (specifically its fields src/func/line) points to the outermost > error_propagate() rather than the place where the error really happened. > It also makes error_abort completely useless because at the point where > the process gets aborted, the interesting information is already lost. > > So I'd really like to restrict the use of error_propagate() to places > where it's absolutely necessary. Unless, of course, you can fix these > practical problems that error_propagate() causes for debugging. > > In fact, in the context of Greg's series, I think we really only need to > support hints for error_fatal, which are cases that users are supposed > to see. We should exclude error_abort in MAKE_ERRP_SAFE() because these > are things that are never supposed to happen. A good stack trace is more > important there than adding a hint to the message. >
Interesting, that to handle error_append_hint problem, we don't need to create local_err in case of errp==NULL either.. So, possibly, we need the following steps: 1. implement MAKE_ERRP_SAFE_FOR_HINT (which only leave "*(errp) == error_fatal" in the if condition 2. rebase Greg's series on it, to fix hints for fatal errors 3. implement MAKE_ERRP_SAFE_FOR_DEREFERENCE (which only leave "(errp) == NULL" in the if condition 4. convert all (almost all) local_err usage to use MAKE_ERRP_SAFE_FOR_DEREFERENCE, which will fix problem with error_abort (and also drop a lot of calls of error_propagate) 5. merely convert "void func(.., errp)" to "int func(.., errp)" and drop MAKE_ERRP_SAFE_FOR_DEREFERENCE() magic, together with dereferencing. -- Best regards, Vladimir