[Nut-upsdev] [nut-commits] svn commit r1136 - in trunk: . drivers

[Alexander I. Gordeev]

On Wed, Oct 03, 2007 at 08:16:23PM +0200, Arjen de Korte wrote:
> Alexander I. Gordeev wrote:
> 
> > You are right it's better to use ser_flush_io for this.
> > But I still disagree on not having a special subdriver function to
> > handle this. (Of course, the current ones should be renamed to, say,
> > flush_<subdriver>).
> 
> Neither the agiler, nor the krauler subdrivers support flushing the
> input buffers. Instead, what happens (for the agiler subdriver) is that
> it will attempt to read from the device until no more data is present.
> Which means that there is (should be) no specific (low level) subdriver
> function to handle this.
> 
[snip]
> 
> >> Don't mistake upsdebugx() messages for error messages. It is telling you
> >> just that no command is set. This is *not* an error and therefor doesn't
> >> need fixing. It is perfectly OK to call the get_data_krauler() function
> >> without setting a command (robustness). Users will never see the output
> >> from the debugging command.
> > I don't agree with you.
> > Yes, it certainly is not an error, it is useless but tolerated debug message.
> 
> You still don't grasp it. Just remove the debug message and you'll have
> the same effect. What I certainly don't want to see in subdrivers, is
> that there is code that is effectively duplicating other code. The
> init() function in the agiler driver is just that. If we ever get
> additional subdrivers, this means that they will have to implement this
> too because you want to remove one upsdebugx() message.
> 

But I don't want to duplicate the code! I just didn't look that far in
the future ;) (Btw, if a new subdriver appears it could need flushing in
a copletely different way. Then it would be simpier to have several
functions for specific _methods_ and pointers to them in the subdriver
definition.)

Ok, I don't mind, let's put it on the "high" level. But please let me

> > This is like arguing about compiler warnings: some people tolerate them and
> > some try to eliminate them all. Well, I'm the latter one. This is just a
> > point of view.
> 
> This is something completely different. Compiler warnings are messages
> that are generated when syntactically there might be something wrong or
> ambiguous in the code. This is not the case here.
> 

Why not?
While usually compiler warnings point to hidden mistakes in the code,
this is not always true. Sometimes they can be ignored safely. Even
more, I've seen highly-optimized code that generated warnings, but it's
author suggested to ignore them because fixing would break it or make it
less efficient.
But it is offtopic.

P.S. Well, maybe there is a problem with our upsdebugx() semantics: we
mix harmless messages and potentially cautionary ones. I'd better introduce
a new one - upswarning().

> > I think this message will make more unclearness (for everybody not in the
> > context) than having subdriver-specific functions. I am even going to comment
> > them :)
> 
> I think it is an utterly bad idea to try to optimize debug messages away.
> 
> Best regards, Arjen

Still please, let me do it. It could be handled nicely. I'll send a
patch.

--
  Alexander