Re: [libvirt] [PATCH v2 02/20] virlog: Introduce virLogOutputNew

>> + >> + ret->logInitMessage = true; >> + ret->f = f; >> + ret->c = c; >> + ret->data = data; > > From future patches I see this can be a file or syslog fd. > > Anyway, because you're relying on the "->c" to be the free function for > ->data, perhaps there should be a check above that causes an error if > "data" was passed as non-NULL, but ->c is NULL; otherwise, in some > future world someone may begin to leak data unexpectedly. I think having non-NULL data with NULL free callback in general is a valid use-case especially if the data is void * and you store integers in it. Anyway, the problem here are stderr and syslog where you have NULL in the close callback and a file descriptor in @data for the former case, which you really do not want to close anyway, and a valid close callback with NULL @data (syslog keeps the file descriptor private) for the latter. While I can imagine having a dummy close callback for stderr which would just return instantly (however I'd rather avoid that), I really don't want to pass an arbitrary pointer to @data for syslog-based output just to satisfy ATTRIBUTE_NONNULL(3), even though it would be ignored by the syslog close callback. Let me know if I misunderstood your thoughts, I'll continue fixing the rest of the patches in the meantime.