Re: [PATCH] script: evaluate errno only if read() sets it

[Karel Zak <kzak@redhat.com>]

On Thu, Jul 02, 2015 at 12:21:02PM +0200, Ruediger Meier wrote:
> I have a question and a comment about my own patch :)

 :-)

I have applied the patch with some small changes:

> On Thursday 02 July 2015, Ruediger Meier wrote:
> 
> > @@ -367,7 +365,7 @@ static void handle_signal(struct script_control
> > *ctl, int fd)
> >
> >  	bytes = read(fd, &info, sizeof(info));
> >  	if (bytes != sizeof(info)) {
> > -		if (errno == EAGAIN)
> > +		if (bytes < 0 && errno == EAGAIN)
> >  			return;
> 
> Should we also return on EINTR here like we do in handle_io()?

It's probably more robust, Fixed.

> And I see some more potential problems. For example see this snippet 
> from handle_io();
> 
> 	bytes = read(fd, buf, sizeof(buf));
> 	if (bytes < 0) {
> 		DBG(IO, ul_debug(" read failed"));
> 		if (errno == EAGAIN || errno == EINTR)
> 			return;
> 		fail(ctl);
> 	}
> 
> We are using errno after DBG(). But in theory printf() or whatever we do 
> in DBG() could change errno. Wouldn't it be a good idea to write 
> generally all DBG macros errno-invariant?

It seems like overkill solution to modify DBG() and always save errno
on all places. I think it's better to care about this locally on place
where you use DBG(). Fixed in script(1).

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com