2 May
2025
2 May
'25
11:11 a.m.
On Fri May 2 11:04:12 2025 +0000, Piotr Caban wrote:
Sure, it's your call. I'm thinking about it as:
- `ntlm_fork` does initialization, if it fails context should not be created
- `ntlm_chat` operates on context, so it's not responsible for clearing
it in case of error
- `ntlm_cleanup` cleanups the context
It's a very common pattern, like e.g. in file operations - you don't expect that failed read will close file descriptor. But you do expect that failed open will not create it. Current code looks very strange for me. We're writing "BH" to the pipe, so I'm expecting that there may be a path that reads it but it's never read.
Right, I see ntlm_fork() as doing part of the initialization and ntlm_chat() doing another part. ntlm_SpInitLsaModeContext() creates the context, not ntlm_fork(), so to me it makes sense to me that the former is responsible for cleanup.
Yes, the "BH" thing is strange. It can probably be removed.