Skip to content
This repository has been archived by the owner on Jun 30, 2020. It is now read-only.

Change write() and read() function to send() and recv() #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DK-DARKmatter
Copy link

Replace the read() and write() function with send() and recv() in
main.c, the flag is set to 0.

Replace the read() and write() function with send() and recv() in
 main.c, the flag is set to 0.
Copy link
Contributor

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good commit message would explain why this change is happening - it's not at all clear why we would want to make this change right now. This is especially true because it breaks the test suite - that needs to pass.

@DK-DARKmatter
Copy link
Author

My bad. I just want to use the CI function, and show what kind of error will happen when we replace those function. I did not want to change current program, but I think if we want to improve the test coverage we should figure out the bugs first.

@npmccallum
Copy link
Contributor

I asked Ke to improve our test coverage. Before he made larger changes, he was just going to swap the IO functions and ensure that everything still worked. It didn't. For some reason, clients fail when using send()/recv() instead of read()/write(). We need to find out why.

@puiterwijk
Copy link
Contributor

puiterwijk commented Mar 25, 2019

@yingxiongraomingzk The problem with your patch is that your code now sends recv() and send() on stdio (buffers 0, 1, 2). That is invalid, as these functions are only allowed for sockets. (stdin, stdout and stderr are non-socket file descriptors.)

The following, brute-force, patch made the tests pass for me:

diff --git a/bin/main.c b/bin/main.c
index 166a4a3..78e2538 100644
--- a/bin/main.c
+++ b/bin/main.c
@@ -254,7 +254,10 @@ on_conn(options_t *opts, int con, int in, int out, const struct addrinfo *ai)
       if (!pfds[i].revents)
         continue;
 
-      ret = read(pfds[i].fd, buffer, sizeof(buffer));
+      if (pfds[i].fd <= 2)
+        ret = read(pfds[i].fd, buffer, sizeof(buffer));
+      else
+        ret = recv(pfds[i].fd, buffer, sizeof(buffer), 0);
       if (ret <= 0) {
         if (pfds[i].revents != POLLHUP &&
             (errno == EAGAIN || errno == EWOULDBLOCK))
diff --git a/bin/non.c b/bin/non.c
index c20db0c..34bbf48 100644
--- a/bin/non.c
+++ b/bin/non.c
@@ -98,7 +98,10 @@ non_write(int fd, void *buf, size_t len)
   uint8_t *b = buf;
   ssize_t ret;
 
-  ret = write(fd, buf, len);
+  if (fd <= 2)
+    ret = write(fd, buf, len);
+  else
+    ret = send(fd, buf, len, 0);
   if (ret < 0) {
     if (errno != EAGAIN)
       return ret;

puiterwijk added a commit to puiterwijk/tlssock that referenced this pull request Mar 25, 2019
This should increase the test coverage to also cover send/recv.

AlternativeTo: enarx-archive#28
Signed-off-by: Patrick Uiterwijk <[email protected]>
@npmccallum
Copy link
Contributor

This PR is superseded by #31 .

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

Successfully merging this pull request may close these issues.

4 participants