rdb.c: handle fclose error case differently to avoid double fclose#7307
rdb.c: handle fclose error case differently to avoid double fclose#7307oranagra merged 1 commit intoredis:unstablefrom
Conversation
hwware
changed the title
There was a problem hiding this comment.
thanks.
So the repeated call to fclose can only happen if the first one failed, right? other errors won't currently cause two calls to fclose. (i.e. could be worse)
regarding the fix, i would rather nullify the fp and add a check in the error handling than duplicate all the termination code.
can you do that change?
let's nullify it at declaration too, so that the goto can be used anywhere in this method and the fclose will be done only if the file is open.
|
thanks @oranagra for your advice. i think this only happens in the fclose error case. I will check other function to see if is there any simiar issue also. Please review my updated commit too. thanks! |
|
@hwware please check the commit i added. also i nullify after close in case another goto will be added. and changed the error check to be non-specific for EOF (although that's the only error it returns, let's check for !success) please tell me if good to be merged. |
use goto statement for handling common error case Update rdb.c generalize error check for file operations fix error handling issue
hwware
force-pushed
the
rdb_fclose_fix
branch
from
378333b to
567c0d3
Compare
|
@oranagra thanks Oran, now the branch is rebased and passed the check. i think it is good to go... |
oranagra
changed the title
…7307) When fclose would fail, the previous implementation would have attempted to do fclose again this can in theory lead to segfault. other changes: check for non-zero return value as failure rather than a specific error code. this doesn't fix a real bug, just a minor cleanup. (cherry picked from commit 323029b)
…edis#7307) When fclose would fail, the previous implementation would have attempted to do fclose again this can in theory lead to segfault. other changes: check for non-zero return value as failure rather than a specific error code. this doesn't fix a real bug, just a minor cleanup.
…edis#7307) When fclose would fail, the previous implementation would have attempted to do fclose again this can in theory lead to segfault. other changes: check for non-zero return value as failure rather than a specific error code. this doesn't fix a real bug, just a minor cleanup. (cherry picked from commit 323029b)
2 participants
This PR fix https://github.com/antirez/redis/issues/7256, fclose should be called only once to avoid undefined behaviour..