[haiku-commits] Re: r40800 - haiku/trunk/src/add-ons/translators/raw

  • From: Ingo Weinhold <ingo_weinhold@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 04 Mar 2011 08:53:49 +0100

On 2011-03-04 at 04:21:53 [+0100], leavengood@xxxxxxxxx wrote:
> Author: leavengood
> Date: 2011-03-04 04:21:53 +0100 (Fri, 04 Mar 2011)
> New Revision: 40800
> Changeset: http://dev.haiku-os.org/changeset/40800
> Ticket: http://dev.haiku-os.org/ticket/6975
> 
> Modified:
>    haiku/trunk/src/add-ons/translators/raw/ReadHelper.h
> Log:
> Apply patch from stimut to avoid a C++ exception in the JPEGTranslator.
> 
> Should fix the remaining issue on #6975.
> 
> 
> Modified: haiku/trunk/src/add-ons/translators/raw/ReadHelper.h
> ===================================================================
> --- haiku/trunk/src/add-ons/translators/raw/ReadHelper.h    2011-03-03 
> 21:56:56 UTC (rev 40799)
> +++ haiku/trunk/src/add-ons/translators/raw/ReadHelper.h    2011-03-04 
> 03:21:53 UTC (rev 40800)
> @@ -76,7 +76,7 @@
>          operator()(T &data)
>          {
>              fError = fStream.Read((void *)&data, sizeof(T));
> -            if (fError > B_OK) {
> +            if (fError >= B_OK) {
>                  if (IsSwapping())
>                      byte_swap(data);
>                  return;

The code is flawed in several ways:

* Mostly aesthetical: When people write ">= B_OK" they usually mean ">= 0". 
Functions like this usually return either a value >= 0 on success or an 
error code on failure. B_OK is never returned, though.

* fError is a status_t. It should not be assigned a ssize_t value. This is 
not only for aesthetical reasons: On 64 bit architectures ssize_t is 
actually wider than status_t.

* Obviously the code expects size(T) bytes of data. The case that less than 
that is read is silently ignored and the caller is left with partial (or 
since this change even) complete garbage.

* The Next() method duplicates the code (it should call this operator 
instead) and still has the old comparison.

CU, Ingo

Other related posts: