[...] >>> + >>> + 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. > Yeah - I think I realized this later too (patch 7)... I guess I was "over"thinking the fd/journalfd usage where we want someone to "forget" somehow to free the resource we're about to "steal" and store. So ignore the whole ATTRIBUTE_NONNULL(3)... John -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list