[Libvir] [PATCH] Enhance virBuffer code

[Daniel Veillard]

On Fri, Dec 14, 2007 at 11:53:35PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> Personally, I don't care at all, but I suspect that people using z/OS
> and OS/390 care, among others.  Some might even be Red Hat customers.
> What's wrong with making the code portable, just in case?

 You can spend a week making the code portable to 0.01% of the user base
and in the process introduce suble problems which may impact far more
than those 0.01%, that's why I think keeping the range checks and adding
a warning about it not being okay on EBCDIC is the best ATM.

> > This just makes it totally unreadable.
> 
> This just makes *what* totally unreadable?
> I proposed to do this:
> 
>     for (p = str; *p; ++p) {
>         is_alphanum (*p)
>     ...
> 
>     for (p = str; *p; ++p) {
>         is_alphanum (*p)
> 
> in place of the code above with all the comparisons.  Or maybe I'm
> misinterpreting something and you're saying the 11-operations-per-byte
> version is harder to read/maintain?  In that case, I would agree.

  The big switch was unreadable.

> Encapsulating the test makes it _more_ readable and maintainable,
> since there is far less syntax, less logic, and less duplication.
>
> If you're saying that the definition of is_alphanum is unreadable,
> I don't buy it.  It's verifiably correct, and works properly even for
> those unfortunate enough to be using EBCDIC.  Why bother to read it when

  You always base your decision criteria on a GNU toolchain, where you
know how to go and look for the definitions of the macros and the code.
I guess a lot of our potential users (and even Red Hat customers) may
be tempted to use libvirt and the related code in completely different
environment, and your decision criteria which are perfectly valid
in a GNU framework may not be that correct there (how do you verify
is_alphanum behaviour w.r.t. locale settings when you are compiling
for example on the Chinese Windows version ? I have no idea, and sometimes
I don't want to take the risk.)
  I think it is good to have different kind of expertise around, as long
as all the inputs can be taken into account to make the final decision.
But that means there will be time where someone viewpoint will have
to be ruled off in order to progress. To me the key point of an open 
source process is that the various points and people are recorded, for
example to be able to appeal or explain if the issue is raised in the 
future.

> it's so simple and automatically generated?  Between an understandable
> name and a comment explaining what it does, I would have thought that'd
> be enough for anyone.

  Well the understandable name of is_* happens to be locale dependant in a
number of cases you can't tell offhand which ones, and that is a serious
side effect which is not clear at all. I have been bitten by that in the
past, and now I'm just cautious about using them when for grammar definition
and not string processing.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/