Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if (! connect(...)) { handle error } #63

Open
ovidiuadam opened this issue Jan 27, 2023 · 16 comments
Open

if (! connect(...)) { handle error } #63

ovidiuadam opened this issue Jan 27, 2023 · 16 comments

Comments

@ovidiuadam
Copy link

Hello, It seems to me that the statement in https://github.com/FastCGI-Archives/fcgi2/blob/master/libfcgi/os_win32.c, function int OS_FcgiConnect(char *bindPath), line 850 should be changed to:
if (connect(sock, (struct sockaddr *) &sockAddr, sockLen) < 0) { ...
Best regards,
Ovidiu Adam

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

i think is ok like is writen!

if (! connect(sock, (struct sockaddr *) &sockAddr, sockLen)) 

in man of connect https://man7.org/linux/man-pages/man2/connect.2.html.
connect return:

  • 0 (connection ok),
  • -1 (connection ko).

in c language (https://c-for-dummies.com/blog/?p=1188)

  • 0 = false,
  • not 0 (>0 or <0) = true.

this if condition try to check if connect is ko...

  • connect return -1 in ko case,
  • and is true (<0),
  • and with ! (reverse the boolean value) is true...
  • and is match the ko case...
  • and go to closesocket and return -1...

if need to bee changed, the prefered condition while be != 0...

if (connect(sock, (struct sockaddr *) &sockAddr, sockLen)!=0) 

@ovidiuadam
Copy link
Author

Hello Mathieu - I beg to differ
The way the current code is written (!connect()) {error}, when connect() succeeds, it returns 0 forcing the execution into handling an error which does not exist, and closing an innocent socket.
For the error case the man says "On error, -1 is returned", hence my suggestion for the "<0 " test
Best regards, Ovidiu

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

no return 0 (equal to false) is inverted and then come false then not entering in closingsocket brace.
and then return anything differant than 0 in the case -1 (equal to true) then inverted is now false, then enter to the brace.

@ovidiuadam
Copy link
Author

.. and yes, changing the test to "the preferred condition while be != 0", will work too. Note that (connect() != 0) is different than (!connect()).

@mcarbonneaux
Copy link
Member

Note that (connect() != 0) is different than (!connect()).

is equivalent, but more readable.

@ovidiuadam
Copy link
Author

Not equivalent. Different. (!connect()) is equivalent to (connect() == 0)

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

no i not ok with that!

  • connect() == 0 is matching connect ok and is equivalent to if (!connect())
  • connect() != 0 or more specificly connect == -1 is matching connect ko and is equivalent to if (connect())

in c language, 0 = false and not 0 is true....

@ovidiuadam
Copy link
Author

No. it is not working. This is how I noticed it.

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

is used in many project.... if the condition not worked any project on unix using this framework while not work at all...

@ovidiuadam
Copy link
Author

ovidiuadam commented Feb 5, 2023

[Ovidiu has revised this post, to match the mcarbonneaux post above. After the edits, the 2 posts are now in agreement]
No, I (Ovidiu) contend that:
connect() == 0 is matching connect success (according to man) is equivalent to if (!connect())
connect() != 0 or more specifically connect == -1 is matching connect failure and is equivalent to if (connect())

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

then if (!connect()) is equivalent to connect() != 0....

@ovidiuadam
Copy link
Author

This happens on Windows, not unix.

@ovidiuadam
Copy link
Author

"0 is false in c langage" . Yes. (!connect()) --> false --> 0 --> and man says that 0 is the return value for success, but the Windows if(!connect()) executes the failure branch, in case of success

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

are you sure that connect on windows return 0 on success ?
this as been more tested on unix...i think...

because this function is the first function that are call when you use this framework... i'm suprise that not work on windows...

on unix part is checking differently.
https://github.com/FastCGI-Archives/fcgi2/blob/master/libfcgi/os_unix.c#L459

connectStatus = connect(resultSock, (struct sockaddr *) &sa.unixVariant, len);
if(connectStatus >= 0) {
    return resultSock;
} 
else {
    /*
     * Most likely (errno == ENOENT || errno == ECONNREFUSED)
     * and no FCGI application server is running.
     */
    close(resultSock);
    return -1;
}

on windows the connect fonction seem to work like on unix:
https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-connect

  • 0 when ok
  • SOCKET_ERROR when ko (seem to be -1)

but the check are not the same (i ok with you but i don't understand why none as send feedback for that):
https://github.com/FastCGI-Archives/fcgi2/blob/master/libfcgi/os_win32.c#L850

if (! connect(sock, (struct sockaddr *) &sockAddr, sockLen)) 
{
	closesocket(sock);
	return -1;
}

but is possible that the most used case on windows is using pipe....

@mcarbonneaux
Copy link
Member

mcarbonneaux commented Feb 5, 2023

i sayed erronous:

the if condition try to check if connect is ko...

when connect return -1 in ko case:

  • and is true (!=0),
  • and with ! (reverse the boolean value) is false...
  • and is not match the ko case...
  • and not go to closesocket and return -1...

when connect return 0 in ok case:

  • and is false (=0),
  • and with ! (reverse the boolean value) is true...
  • and is match the ok case...
  • and go to closesocket and return -1...

and simple (connect()) or (connect()!=0) or (connect()<0) (like you sayed) while be ok.

@ovidiuadam
Copy link
Author

ovidiuadam commented Feb 5, 2023

Bingo! - Yes, connect() in Windows works the same as in unix (it returns 0 for success and -1 for error).
Thank you, Ovidiu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants