Re: [E-devel] E SVN: pfritz trunk/ecore/src/lib/ecore

2008-11-03 Thread Gustavo Sverzut Barbieri
On Mon, Nov 3, 2008 at 11:57 AM, Enlightenment SVN
[EMAIL PROTECTED] wrote:
 +   /* first write the len into the pipe */
 +   do
 + {
 +   ret = pipe_write(p-fd_write, nbytes, sizeof(nbytes));
 +   if (ret == sizeof(nbytes))
 + {
 +retry = ECORE_PIPE_WRITE_RETRY;
 +break;
 + }
 +   else if (ret  0)
 + {
 +/* XXX What should we do here? */
 +fprintf(stderr, The length of the data was not written complete
 +  to the pipe\n);

try to read what's missing, you'll end with EAGAIN if there is no data
yet. In that case you either try a busy loop or you save number of
read bytes and what's missing and continue on next next fd_handler.

 +   else
 + {
 +fprintf(stderr, An unhandled error (ret: %d errno: %d)
 +  occured while writing to the pipe the length\n,
 +  ret, errno);

please, use strerror(errno) to make it easier to figure out what's happened.


 + }
 + }
 +   while (retry--);
 +
 +   if (retry != ECORE_PIPE_WRITE_RETRY)
 + return 0;

this termination condition is weird, if it's retry it should mean we
should retry, not that it finished fine.

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--
MSN: [EMAIL PROTECTED]
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202

-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK  win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100url=/
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] E SVN: pfritz trunk/ecore/src/lib/ecore

2008-11-03 Thread Peter Wehrfritz

Gustavo Sverzut Barbieri schrieb:

On Mon, Nov 3, 2008 at 11:57 AM, Enlightenment SVN
[EMAIL PROTECTED] wrote:
  

+   /* first write the len into the pipe */
+   do
+ {
+   ret = pipe_write(p-fd_write, nbytes, sizeof(nbytes));
+   if (ret == sizeof(nbytes))
+ {
+retry = ECORE_PIPE_WRITE_RETRY;
+break;
+ }
+   else if (ret  0)
+ {
+/* XXX What should we do here? */
+fprintf(stderr, The length of the data was not written complete
+  to the pipe\n);



try to read what's missing, you'll end with EAGAIN if there is no data
yet. In that case you either try a busy loop or you save number of
read bytes and what's missing and continue on next next fd_handler.

  
It is actually a case that should never happen, i think. Anyway I added 
code to pass the rest of the length again.

+   else
+ {
+fprintf(stderr, An unhandled error (ret: %d errno: %d)
+  occured while writing to the pipe the length\n,
+  ret, errno);



please, use strerror(errno) to make it easier to figure out what's happened.
  

k

+ }
+ }
+   while (retry--);
+
+   if (retry != ECORE_PIPE_WRITE_RETRY)
+ return 0;



this termination condition is weird, if it's retry it should mean we
should retry, not that it finished fine.
  


Indeed, it was not very readable. I changed the logic abit.

Index: ecore_pipe.c
===
--- ecore_pipe.c	(Revision 37441)
+++ ecore_pipe.c	(Arbeitskopie)
@@ -326,7 +326,8 @@
 	  ecore_pipe_del);
 	return NULL;
  }
-   ecore_main_fd_handler_del(p-fd_handler);
+   if (p-fd_handler)
+ ecore_main_fd_handler_del(p-fd_handler);
close(p-fd_read);
close(p-fd_write);
data = (void *)p-data;
@@ -348,7 +349,13 @@
ssize_t ret;
size_t already_written = 0;
int retry = ECORE_PIPE_WRITE_RETRY;
+   union {
+	unsigned int isize;
+	unsigned char bytes[sizeof(nbytes)];
+   } size;
 
+   size.isize = nbytes;
+
if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
  {
 	ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE,
@@ -358,35 +365,30 @@
/* first write the len into the pipe */
do
  {
-	ret = pipe_write(p-fd_write, nbytes, sizeof(nbytes));
-	if (ret == sizeof(nbytes))
-	  {
-	 retry = ECORE_PIPE_WRITE_RETRY;
-	 break;
-	  }
-	else if (ret  0)
-	  {
-	 /* XXX What should we do here? */
-	 fprintf(stderr, The length of the data was not written complete
-		   to the pipe\n);
-	 return 0;
-	  }
+	ret = pipe_write(p-fd_write, size.bytes[already_written],
+sizeof(nbytes) - already_written);
+	if (ret == (ssize_t)(sizeof(nbytes) - already_written))
+	  break;
+	else if (ret = 0)
+	  already_written += ret;
 	else if (ret == -1  errno == EINTR)
 	  /* try it again */
 	  ;
 	else
 	  {
-	 fprintf(stderr, An unhandled error (ret: %d errno: %d)
+	 fprintf(stderr, An unhandled error (ret: %d errno: %s)
 		   occured while writing to the pipe the length\n,
-		   ret, errno);
+		   ret, strerror(errno));
 	  }
  } 
while (retry--);
 
-   if (retry != ECORE_PIPE_WRITE_RETRY)
+   if (retry  0)
  return 0;
 
/* and now pass the data to the pipe */
+   already_written = 0;
+   retry = ECORE_PIPE_WRITE_RETRY;
do
  {
 	ret = pipe_write(p-fd_write, 
@@ -397,7 +399,7 @@
 	  return 1;
 	else if (ret = 0)
 	  {
-	 already_written -= ret;
+	 already_written += ret;
 	 continue;
 	  }
 	else if (ret == -1  errno == EINTR)
@@ -405,9 +407,9 @@
 	  ;
 	else
 	  {
-	 fprintf(stderr, An unhandled error (ret: %d errno: %d)
+	 fprintf(stderr, An unhandled error (ret: %d errno: %s)
 		   occured while writing to the pipe the length\n,
-		   ret, errno);
+		   ret, strerror(errno));
 	  }
  } 
while (retry--);
@@ -435,28 +437,48 @@
 	 */
 	if (p-len == 0)
 	  {
-	 /* read the len of the passed data */
-	 ret = pipe_read(p-fd_read, p-len, sizeof(p-len));
+	 union {
+		  unsigned int isize;
+		  unsigned char bytes[sizeof(unsigned int)];
+	 } size;
+	 size_t already_read_len = 0;
 
-	 /* catch the non error case first */
-	 if (ret == sizeof(p-len))
-	   ;
-	 else if (ret  0)
+	 for (;;)
 	   {
-		  /* XXX What should we do here? */
-		  fprintf(stderr, Only read %d bytes from the pipe, although
-			 we need to read %d bytes.\n, ret, sizeof(p-len));
+		  /* read the len of the passed data */
+		  ret = pipe_read(p-fd_read, size.bytes[already_read_len], 
+			sizeof(p-len) - already_read_len);
+
+		  /* catch the non error case first */
+		  if (ret == sizeof(p-len))
+		{
+		   p-len = size.isize;
+		   break;
+		}
+		  else if (ret  0)
+		already_read_len += ret;
+		  else if (ret == 0 
+			|| (ret == -1  (errno == EINTR || errno == EAGAIN)))
+		{
+		   if (!already_read_len)
+			 return ECORE_CALLBACK_RENEW;
+		}
+		  else
+		{
+