[Nut-upsdev] [PATCH] al175: updated driver, please restore it

[Arjen de Korte]

Citeren Kirill Smelkov <kirr at mns.spb.ru>:

[...]

> Yes, "All the world is not an x86 Linux box," and I've tried to make all the
> world happy.

I raised several other issues as well, of which the most important one  
isn't addressed. The driver still uses 'alloca' which isn't portable  
(it is not in POSIX). I also indicated how this could be solved by  
using automatic variables. In any case I'm not conviced there is no  
alternative here, so this is a show stopper for now.

Also something I really don't like in this driver is the use of  
alarm(). Although this is presently used in the 'net-xml' driver (in  
case an older neon library is detected), there is no need for this  
here. We have timeouts on *all* ser_get* functions and these *must* be  
used instead. Note that with using timeouts I mean something different  
than setting them to '999' seconds and setting an alarm() to breakout  
early. Unless there is a really good reason to use an alarm() (ie,  
there is no alternative), this shouldn't be used.

Even in the updated driver, I see quite a couple of fprintf() lines.  
Although these are in most (all?) cases encased in a 'if  
(nut_debug_level > 0)', we have the upsdebug* functions for that. Do  
yourself (and your fellow developers) a favor and use those instead.  
Another thing related to this is the XTRACE macro. This will only use  
a single (1) debug level to show output. Please use a layered  
approach, where various levels of debugging information are used. It  
also helps understanding the driver if you keep the number of macro's  
limited.

Best regards, Arjen
-- 
Please keep list traffic on the list