Re: [libvirt] [PATCH v3 5/6] virFileInData: preserve errno in cleanup path

On 06/05/2017 04:23 AM, Michal Privoznik wrote: One could argue that if the original lseek fails, rather than the goto cleanup, the return -1 could be done. That way, the "if" condition in cleanup: isn't necessary... Additionally, if the second if the second lseek fails to reposition back to where we started and we don't tell the user that, then we're off in never never land possibly, right? Of course IIRC the argument about this was that lseek is highly unlikely to fail in that second scenario. Up to this point we don't say/guarantee anything about the errno return value generally, but I see the subsequent patch adds some verbiage - whether it can be accurately held over any libvirt call perhaps remains to be seen. Still the premise of this is, don't mess with an errno that was set to something with a second one that could change the original intention. Before the ACK/R-B applies, is there any odd special casing that needs to be done for that ENXIO case? That is setting errno = 0 if we decide to not goto cleanup as I don't think in that case we're considering an errno value to be problematic, instead it'd be expected. As long as the errno != ENXIO case is handled... Since we learned nothing and are moving on happily. Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John