Return to Answer

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: http://stackoverflow.com/a/8474123/14065

Try this:

static ssize_t read_all(int fdes, void* buffer, ssize_t size)
{
 ssize_t totalRead = 0;
 while(totalRead != size)
 {
 ssize_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTR)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: https://stackoverflow.com/a/8474123/14065

Try this:

static ssize_t read_all(int fdes, void* buffer, ssize_t size)
{
 ssize_t totalRead = 0;
 while(totalRead != size)
 {
 ssize_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTR)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: http://stackoverflow.com/a/8474123/14065

Try this:

static size_t read_all(int fdes, void* buffer, size_t size)
{
 size_t totalRead = 0;
 while(totalRead != size)
 {
 size_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTR)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: http://stackoverflow.com/a/8474123/14065

Try this:

static ssize_t read_all(int fdes, void* buffer, ssize_t size)
{
 ssize_t totalRead = 0;
 while(totalRead != size)
 {
 ssize_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTR)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: http://stackoverflow.com/a/8474123/14065

Try this:

static size_t read_all(int fdes, void* buffer, size_t size)
{
 size_t totalRead = 0;
 while(totalRead != size)
 {
 size_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTER)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}

My main problem is the recursion. I don't think its major but I personally would use a loop. Given the current layout I can't quite convince myself that it works in all situations (especially since there are two alternative recursive calls).

Declare variables close to the usage point (rather than everything at the top).

 ssize_t ret;
 ssize_t ret2;

C has been updated so that you can declare variables at any point in a function. This helps in readability as you don't need to scroll far to find the variable declaration. Also if you declare in the most restrictive scope possible it helps to prevent data leaking (out of a scope).

see: http://stackoverflow.com/a/8474123/14065

Try this:

static size_t read_all(int fdes, void* buffer, size_t size)
{
 size_t totalRead = 0;
 while(totalRead != size)
 {
 size_t thisRead = read(fdes, buffer + totalRead, size - totalRead);
 if ((thisRead == -1) && (errno == EINTR)) {
 continue;
 }
 // Note: There are other errors that may not be erros.
 // EAGAIN or EWOULDBLOCK spring to mind but may need special handling
 // as immediately calling read may just cause a busy wait you may
 // want to suspend the thread by calling sleep..
 if (thisRead == -1) {
 return -1;
 }
 if (thisRead == 0) {
 break;
 }
 totalRead += thisRead;
 }
 return totalRead;
}