🚀 go-pugleaf

Everybody knows nothing is perfect, including programs. I guess
anyone working with BC++ can tell you a story about exotic creatures
he or she have seen in seams and crevices of this wonderful environment.
Here I describe two quite serious bugs -- one in OWL streamable
classes, the other in BIDS library, and one not serious, but quite a
funny one -- in Windows startup code. Those bugs present in BC++ 
version 3.1 and previous.

I'm not sure if I'm right posting this report to comp.windows.ms.programmer
and comp.os.ms-windows.programmer.tools, but I haven't found any group
which is more appropriate. Please re-post if you know one. Maybe I should
send bug report to Borland, but I don't know how to reach them by email,
while email is the only way I can use for communications.

I would be glad to get any response, suggestions or just a chit-chat from
anybody. Send it to Internet: vb@re.mipt.su.

==========================================
OWL Streamable classes -- both 3.0 and 3.1
==========================================

Using OWL streamable classes I had a problem and tracking it down had lead
to a strange implementation feature of classes ipstream and opstream,
looking much like a bug...

ipstream::freadBytes( void far *data, size_t sz ) is implemented in the
following way:

// --- quote from OBJSTRM.CPP --- //

void ipstream::freadBytes( void far *data, size_t sz )
{
    if (sz > 0)
    {
        char *buf = new char[sz+1];

        bp->sgetn( (char *)buf, sz );
        _fstrncpy((char far *)data, buf, sz);

        delete buf;
    }
}

// --- end of quote --- //

According to docs, it "reads the number of bytes specified by sz into the
supplied far buffer (data)".

As you can see, the bytes are first read into a temporary local buffer and
then copied into the supplied far buffer. But instead of _fmemcpy, as it
would be natural, the copying is done by _fstrncpy. It means that ONLY
null-terminated strings are handled by this function as it is stated.  Raw
binary data, in a general case, will be transferred only partially, until
the first of '\0's is encountered. However, according to docs (and to the
meaning of words "read bytes") it is unlikely that this function is
intended for reading only null-terminated strings.

The bug has a mate (even more dangerous) in opstream::fwriteBytes. Look:

// --- quote from OBJSTRM.CPP --- //

void opstream::fwriteBytes( const void far *data, size_t sz )
{
    if (sz > 0)
    {
        char *buf = new char[sz+1];

        _fstrcpy(buf, (char far *)data);
        bp->sputn( (char *)buf, sz );

        delete buf;
    }
}

// --- quote end --- //

Again, according to the docs, it "writes the specified number of bytes
(sz) from the supplied far buffer (data) to the stream". But the bytes are
first copied into a temporary local buffer with a plain _fstrcpy!  It
doesn't only mean that if there's a '\0' byte somewhere in the middle,
we'll copy only the preceding bytes. Just imagine if we have no '\0' bytes
at all! _fstrcpy will wander through the memory, writing PAST THE END of
the allocated buffer until (the best case) it leaves a segment and causes
13th exception or (the worst case) it stops on some '\0', possibly having
quietly corrupted some data residing past the allocated buffer. If the
corruption causes a problem later, imagine how you track this bug down to
its origin...

So, as you can see, both of these member functions are "partially
functional". They work fine with null-terminated strings, but fail to
work with a raw binary data. To make them do what they should, calls to
_fstrncpy and _fstrcpy should be replaced with _fmemcpy.

Their non-far siblings, ipstream::readBytes and opstream::writeBytes work
fine, really reading and writing raw binary data as well as
null-terminated strings.

Streamable classes themselves use these functions only in
ipstream::freadString and opstream::fwriteString. Since the data those
functions operate with ARE null-terminated strings, they have no problems.

I have discovered this problem in BC++ 3.0, but it is present in 3.1 as
well.

============
BIDS library
============

Namely, there's a problem with member functions forEach, firstThat and
lastThat in classes BI_DequeAsVector, BI_QueueAsVector and their indirect
(BI_I...) counterparts. (Later here only direct versions are discussed,
the problems with indirect ones are similar.)

The hierarchy of these classes is as follows:

BI_QueueAsVector<T>
	|
BI_DequeAsVector<T>
	|
BI_DequeAsVectorImp<BI_VectorImp<T>,T>

Queue and deque are derived from class BI_DequeAsVectorImp<class Vect,
class T>, which implements the behavior of queues/deques as vector.  The
first of its parameters, 'Vect', specifies a class used as a vector to
store objects of type specified by the second parameter, 'T'.

In our case BI_DequeAsVectorImp uses member 'data' of type BI_VectorImp<T>
to store a vector of objects of class T. It also has two data members,
'left' and 'right'. These members are the indices of the two ends of a
deque. To store an object at the left end, BI_DequeAsVectorImp decrements
'left', wraps it if needed and then stores the object in data[left]. To
store an object at the right end BI_DequeAsVectorImp stores the object in
data[right], then increments 'right', wrapping if needed. Initially left =
0 and right = 0.

If we need to access the contained objects (as we do in member functions
forEach, firstThat and lastThat, implemented in BI_DequeAsVector) we
should take into account the two possibilities:

left < right	The contained objects reside between left and right-1:
		left <= obj && obj < right

left > right	The contained objects reside at the vector positions
		excluding the range between right-1 and left:
		(0 <= obj && obj < right) || (left <= obj && obj < top)

Any of the above possibilities can take place. Nevertheless, all 
BI_DequeAsVector does is:

// --- quote from BI_DequeAsVector<T> (DEQUES.H) --- //
// ...
void forEach( void (_FAR *f)(T _FAR &, void _FAR *), void _FAR *args )
	{ data.forEach( f, args, left, right ); }

T _FAR *firstThat( int (_FAR *f)(const T _FAR &, void _FAR *),
	void _FAR *args ) const
	{ return data.firstThat( f, args, left, right ); }

T _FAR *lastThat( int (_FAR *f)(const T _FAR &, void _FAR *),
	void _FAR *args ) const
	{ return data.lastThat( f, args, left, right ); }
// ...
// --- end of quote --- //

Thus, only the first possibility is taken into account. In the second case,
when left > right, we won't iterate at all. The following example 
illustrates the problem:

// --- example --- //

#include <iostream.h>
#include <queues.h>

void PrintInt( int & theInt, void* )
  { cout << theInt << '\n'; }

int main()
  {
  BI_QueueAsVector<int> intQueue;	// Queue of ints of default size.
  intQueue.put( 1 );			// Insert a couple
  intQueue.put( 2 );			// of integers.
  intQueue.forEach( PrintInt, NULL );	// Now try to print the contents.
  return 0;
  }

// --- end of example --- //

We could expect the above program to print
1
2
	or
2
1
but actually it prints nothing. It happens because initially
BI_QueueAsVector have left == 0 and right == 0. The objects are inserted
at the 'left' side, so 'left' pointer is wrapped to the vector's top and
we have left > right. When we call forEach, it calls
BI_VectorImp<int>::forEach, specifying 'left' as a starting index and
'right' as ending one. BI_VectorImp<int>::forEach sees that starting index
is greater than the ending and doesn't iterate at all. So PrintInt() never
gets called.

To solve the problem, forEach and firstThat, for example, should be
implemented in class BI_DequeAsVector<T> as follows. (Or you can derive a
class from BI_QueueAsVector or BI_DequeAsVector and overload these member
functions with the correct implementations.)

// --- correct implementation --- //

template <class T>
void BI_DequeAsVector<T>::forEach(void (*f)(T _FAR &, void*), void* args)
  {
  if( left < right )
    data.forEach( f, args, left, right );
  else
    {
    data.forEach( f, args, 0, right );
    data.forEach( f, args, left, data.limit() );
    }
  }

template <class T>
T _FAR * BI_DequeAsVector<T>::firstThat(int (*f)(T _FAR &, void*), void* args)
  {
  if( left <= right )
    return data.firstThat( f, args, left, right );
  else
    {
    T _FAR * ret = data.firstThat( f, args, left, data.limit() );
    if( !ret )
      ret = data.firstThat( f, args, 0, right );
    return ret;
    }
  }

// --- end of correct implementation --- //

====================
Windows startup code
====================

There's a little bug in startup code for Windows -- both application's and
DLL's. (Files C0W.OBJ and C0D.OBJ, their sources are C0W.ASM and C0D.ASM).

The bug is not dangerous and not very important, but it is funny how long
such "childish" bugs can live: I discovered it in Borland C++ 2.0, then
saw it in BC++ 3.0, and later we have met as old good friends in BC++ 3.1

The sense is: if you test global variable _8087 to determine if the math
coprocessor is present on the machine your program is running on, you'll
get 0 -- not present -- always, even if the chip is here. The variable is
set by the startup code as follows:

; --- quote from C0W.ASM --- ;

                call    GETWINFLAGS

; ... testing for protected mode, omitted here ...

;Test for 8087 presence

                test    dx,04h          ; WF_8087 = 0x0400
                jz      @@no8087
                mov     __8087, 1
@@no8087:

; --- end of quote --- ;

GetWinFlags() returns DWORD (i.e. loword in AX, hiword in DX), and WF_8087
is #defined as 0x0400, so we need to

	test	ah, 04h
or
	test	ax, 0400h

Thus, the code actually tests the returned value against 0x40000. This bit
is currently not used (or, at least, is not documented) and always clear,
so _8087 is always set to 0.

(Though I seem there are few people who could be concerned.)