Relay-Version: version B 2.10 5/3/83; site utzoo.UUCP
Posting-Version: version B 2.10.2 8/7/84; site ucbvax.ARPA
Path: utzoo!utcs!lsuc!pesnta!hplabs!ucbvax!anton
From: anton@ucbvax.ARPA (Jeff Anton)
Newsgroups: net.bugs.4bsd
Subject: Re: Ingres bug in 4.2 BSD
Message-ID: <4822@ucbvax.ARPA>
Date: Thu, 14-Feb-85 13:26:49 EST
Article-I.D.: ucbvax.4822
Posted: Thu Feb 14 13:26:49 1985
Date-Received: Fri, 15-Feb-85 02:12:57 EST
References: <220@tellab2.UUCP>
Reply-To: anton@ucbvax.UUCP (Jeff anton)
Organization: University of California at Berkeley
Lines: 62
Summary: 

In article <220@tellab2.UUCP> steve@tellab3.UUCP (& Harpster) writes:
>
>Subject: Retrieving long relation returns bad character count
>Index:	~ingres/source/libq/IIn_ret.c 4.2BSD
>
>Description:
>	Occasionally, a C program (run through equel) which is trying to
>	retrieve tuples from an Ingres database returns the message
>	"IIn_ret: bad pb_get-1 <# bytes read>".
>Repeat-By:
>	Create a relation with a character domain of length 255 (the
>	maximum). Now have a C program try and retrieve a tuple from it.
>Fix:
>	Simple type conflicts. Casts fix the problem. The diffs are below:
>
>*** /tmp/old_IIn_ret.c	Tue Feb 12 13:22:31 1985
>--- /user2/ingres/source/libq/IIn_ret.c	Tue Feb 12 12:50:28 1985
>***************
>*** 46,53
>          ret = &IIr_sym;
>  
>  	if (ret->len &= 0377)
>! 		if (IIpb_get(&IIpb, temp, ret->len & 0377) != ret->len & 0377)
>! 			IIsyserr("IIn_ret: bad pb_get-1 %d", ret->len & 0377);
>  
>  
>  #	ifdef xETR1
>
>--- 46,54 -----
>          ret = &IIr_sym;
>  
>  	if (ret->len &= 0377)
>! 		if ((length = IIpb_get(&IIpb, temp, ret->len & 0377)) != (int) (ret->len & 0377))
>! 			IIsyserr("IIn_ret: bad pb_get-1, wanted %d got %d",
>! 				(int) (ret->len & 0377), length);
>  
>  
>  #	ifdef xETR1

The problem with the original code was not typeing but operator precidence.
The '!=' would bind more causeing ret->len to be signextended to int
and the comparison fails; then the result of the compairison is bit wise anded
with 0377.  There are many strange things about this code.  For example,
take the first if statement.  'if (ret->len &= 0377)'  What is this supposed
to do?  ret->len is a char so &= with 0377 is a futile operation.
Below is a much clearer version:
	if (ret->len != 0 &&
	    IIpb_get(&IIpb, temp, ret->len & I1MASK) != (ret->len & I1MASK))
		IIsyserr("IIn_ret: bad pb_get-1, %d", ret->len & I1MASK);

I1MASK is defined in h/ingres.h

nontrivial question: Would casting ret->len as unsigned work better,
not work, look better, or look worse?  I know the answer.

-- 
_________________________
C knows no bounds.
					Jeff Anton
					U.C.Berkeley
					ucbvax!anton
					anton@berkeley.ARPA