apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Karl Fogel <kfo...@newton.ch.collab.net>
Subject Re: useless return values for apr_md5_*() functions...
Date Fri, 12 Apr 2002 21:09:58 GMT
"Bill Stoddard" <bill@wstoddard.com> writes:
> Breaking API compatability is not something that should be taken
> lightly.  This is an issue with all users of APR, not just the http
> server. I am not against the API changing if there is a compelling
> reason for it. This change is light-years from compelling.

Well, now, I don't know if it's all that many light years... :-)

IMHO, this is essentially a documentation bug.

The return codes are confusing to callers.  Someone computing MD5 is
likely to wonder what the heck could possibly go wrong, since in
theory *nothing* can go wrong.  Checksum computation on arbitrary data
has to be foolproof, assuming the buffer is long enough.  Being able
to return an error from the computation is equivalent to having a bug
in the checksumming code, really.  And instead of returning error, we
would just fix the bug.

So the presence of the return values sends the developer into the
source code to see why there is this puzzling possibility of error.
Whereupon he discovers that things are exactly as he suspected -- the
functions really *can't* return any errors.

Well, if they can't return errors, the calling code certainly doesn't
need to check the return values, and so he writes code that tosses the
return values.  Or he doesn't.  Either way is bad.

Now the next person comes along.  She sees the code, maybe has seen
other code that uses the same interface, and wonders why one place
checks the return values and the other doesn't.

She makes the same inspection.  Or, she asks on the list.  Now a bunch
of people go diving into the sources, and discover the same thing...

You get the idea.  A lot of wasted time and mental energy.

Offering an error code from an operation that can't possibly error is
not doing anyone a favor.  It's confusing, and leads to inconsistent
calling code.  (Not to mention that for those who trust the .h file
blindly, their code gets cluttered with error checks that they don't
need to be doing.)

Is it a major bug?  No.

Should it be fixed?  Yes, I certainly think so.  IMHO, this is not
"light-years from compelling", it's something that needs to get done
eventually, to avoid wasting people's time.

> Breaking API compatability is not something that should be taken
> lightly.

Not at all -- light breaking *can* be taken lightly, because it's not
going to make much work for people.  Serious breaking should be taken
more seriously, of course, but this isn't that kind of change.

"Serious" breaking would be adding arguments to a function, or
changing types, because that makes real work -- people have to prepare
objects that they didn't have to prepare before.  But getting rid of a
return value?  It just means people get some perfectly clear compiler
warnings, and remove some error checks to fix them.

That's hardly an onerous burden, I think. :-)

-Karl

Mime
View raw message