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)

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=100&url=/
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel