machinezone / IXWebSocket

websocket and http client and server library, with TLS support and very few dependencies
BSD 3-Clause "New" or "Revised" License
512 stars 167 forks source link

Don't call close() on -1. #502

Closed CryptoManiac closed 3 months ago

CryptoManiac commented 4 months ago

Valgrind keeps complaining that close() on the invalid descriptor -1 is happening here. I suppose that close shouldn't be called when the descriptor value is known to be invalid. It's not a fatal error or something, but this practice is able to create a lot of flood in the logs.

bsergean commented 3 months ago

Thanks ! I usually put the -1 on the right hand side but that's fine.

CryptoManiac commented 3 months ago

Thanks ! I usually put the -1 on the right hand side but that's fine.

Putting constants on the left side of comparison is a compiler-agnostic way to prevent accidental assignment related bugs. Search for the term Yoda’s notation for more info.

bsergean commented 3 months ago

I had forgotten that Yoda denomination.

Could you flip it the normal way? I like the code to stay consistent with the classical way.

On Tue, Mar 19, 2024 at 11:33 AM CryptoManiac @.***> wrote:

Thanks ! I usually put the -1 on the right hand side but that's fine.

Putting constants on the left side of comparison is a compiler-agnostic way to prevent accidental assignment related bugs. Search for the term Yoda’s notation for more info.

— Reply to this email directly, view it on GitHub https://github.com/machinezone/IXWebSocket/pull/502#issuecomment-2007870460, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2O6UIMFZRF6GPAISXLVHLYZCAF7AVCNFSM6AAAAABDXE5DOSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBXHA3TANBWGA . You are receiving this because you modified the open/close state.Message ID: @.***>

CryptoManiac commented 3 months ago

I had forgotten that Yoda denomination. Could you flip it the normal way? I like the code to stay consistent with the classical way.

Sure, I'll revise this tomorrow I think.