Citeren Kirill Smelkov <[email protected]>: [...]
> 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 _______________________________________________ Nut-upsdev mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
