salzman%gaucho@RAND.ORG (Isaac Salzman) (11/16/89)
[this was part of one long message - i decided to split it into several
smaller messages]
hi there. the following is in reference to Andrew R4 Beta release, patch
level 5 running on a Sparcstation-1 (SunOS 4.0.3c).
i've discovered a bug in the wp code on SPARC. actually, it's a bug in
Sun's SPARC C compiler that's exercised by wp. it's in wp_DeAllocate()
in overhead/util/lib/wpbase.c, which is declared as follows:
wp_ErrorCode wp_DeAllocate(StrPtr)
union { wp_PrimeKeySet *PKSet; struct wp_SrchToken *ST; wp_SearchToken
XX;} StrPtr;
{
/* stuff */
}
Sun's C compiler doesn't like the union. it compiles ok, but gets a bus
error when you try to run it (when called from wpq for instance). this
works ok on a Sun3, or a Sun4 with the GNU C Compiler (gcc). i'm going
to send a bug report to sun. i don't suspect a fixed compiler will happen
anytime soon, so i've hacked the code to pass StrPtr as a char *, and then
cast it to the apropriately (e.g. ((struct xxx *)StrPtr)->yyy). context
diff's enclosed.
i've noticed a new .tar.Z file and emsworth. i gather that's the "release"
version? i've grabbed patch06 but haven't applied it (it doesn't fix the wp
problem - i checked). ciao!
* Isaac J. Salzman ----
* The RAND Corporation - Information Sciences Dept. /o o/ /
* 1700 Main St., PO Box 2138, Santa Monica, CA 90406-2138 | v | |
* AT&T : +1 213-393-0411 x6421 or x7923 (ISL lab) _| |_/
* Internet : salzman@rand.org / | |
* UUCP : !uunet!rand.org!salzman | | |
* CompuServe: 76167,1046
---- Enclosure ----
*** Orig/wpbase.c Wed Nov 15 08:03:12 1989
--- wpbase.c Wed Nov 15 08:14:07 1989
***************
*** 363,408 ****
Clean up after the client is done with malloc'ed storage.
*/
wp_ErrorCode wp_DeAllocate(StrPtr)
! union { wp_PrimeKeySet *PKSet; struct wp_SrchToken *ST; wp_SearchToken
XX;} StrPtr;
{
int ctr;
struct wp_Constraint *CPtr;
! if (StrPtr.PKSet == NULL) return wperr_NoError;
#if Logs
Log(700, "wp_DeAllocate(0x%x) called", StrPtr);
#endif /* Logs */
! if (StrPtr.PKSet->KeyCount >= 0) { /* it's a pointer to
PrimeKeySetStr */
if (wp_Debugging) {fprintf(stderr,
"wp_DeAllocate(%#x) releasing a %d-key PrimeKeySet.\n",
! StrPtr.PKSet, StrPtr.PKSet->KeyCount);
}
! if (StrPtr.PKSet->Keys != NULL) {
if (wp_Debugging) fprintf(stderr, "PrimeKeySet has keys at %#x.\n",
! StrPtr.PKSet->Keys);
! for (ctr = StrPtr.PKSet->KeyCount - 1; ctr >= 0; --ctr) {
! if (StrPtr.PKSet->Keys[ctr] != NULL) {
if (wp_Debugging) fprintf(stderr,
! "Releasing PKset key[%d]=%#x.\n",
! ctr, StrPtr.PKSet->Keys[ctr]);
! free(StrPtr.PKSet->Keys[ctr]);
}
}
! free(StrPtr.PKSet->Keys);
}
! free(StrPtr.PKSet);
! } else if (StrPtr.ST->Tag == SrchTokenTag) {
! CPtr = StrPtr.ST->Constraints;
while (CPtr != NULL) {
! StrPtr.ST->Constraints = CPtr->Next;
if (CPtr->Tag != ConstraintTag) return wperr_TokenMalformed;
free(CPtr->FieldContent);
free(CPtr);
! CPtr = StrPtr.ST->Constraints;
}
! free(StrPtr.ST->Probe);
! free(StrPtr.ST);
} else return wperr_NoSuchTokenKind;
if (wp_Debugging) fprintf(stderr, "wp_DeAllocate returning.\n");
#if Logs
--- 363,415 ----
Clean up after the client is done with malloc'ed storage.
*/
wp_ErrorCode wp_DeAllocate(StrPtr)
! char *StrPtr;
!
! /* union { wp_PrimeKeySet *PKSet; struct wp_SrchToken *ST;
! wp_SearchToken XX;} StrPtr;
! */
!
{
int ctr;
struct wp_Constraint *CPtr;
! if (StrPtr == NULL) return wperr_NoError;
#if Logs
Log(700, "wp_DeAllocate(0x%x) called", StrPtr);
#endif /* Logs */
! if ( ((wp_PrimeKeySet *)StrPtr)->KeyCount >= 0) { /* it's a
pointer to PrimeKeySetStr */
if (wp_Debugging) {fprintf(stderr,
"wp_DeAllocate(%#x) releasing a %d-key PrimeKeySet.\n",
! (wp_PrimeKeySet *)StrPtr,
! ((wp_PrimeKeySet *)StrPtr)->KeyCount);
}
! if (((wp_PrimeKeySet *)StrPtr)->Keys != NULL) {
if (wp_Debugging) fprintf(stderr, "PrimeKeySet has keys at %#x.\n",
! ((wp_PrimeKeySet *)StrPtr)->Keys);
! for (ctr = ((wp_PrimeKeySet *)StrPtr)->KeyCount - 1; ctr >= 0;
! --ctr) {
! if (((wp_PrimeKeySet *)StrPtr)->Keys[ctr] != NULL) {
if (wp_Debugging) fprintf(stderr,
! "Releasing PKset key[%d]=%#x.\n", ctr,
! ((wp_PrimeKeySet *)StrPtr)->Keys[ctr]);
! free(((wp_PrimeKeySet *)StrPtr)->Keys[ctr]);
}
}
! free(((wp_PrimeKeySet *)StrPtr)->Keys);
}
! free((wp_PrimeKeySet *)StrPtr);
! } else if (((struct wp_SrchToken *)StrPtr)->Tag == SrchTokenTag) {
! CPtr = ((struct wp_SrchToken *)StrPtr)->Constraints;
while (CPtr != NULL) {
! ((struct wp_SrchToken *)StrPtr)->Constraints = CPtr->Next;
if (CPtr->Tag != ConstraintTag) return wperr_TokenMalformed;
free(CPtr->FieldContent);
free(CPtr);
! CPtr = ((struct wp_SrchToken *)StrPtr)->Constraints;
}
! free(((struct wp_SrchToken *)StrPtr)->Probe);
! free(((struct wp_SrchToken *)StrPtr));
} else return wperr_NoSuchTokenKind;
if (wp_Debugging) fprintf(stderr, "wp_DeAllocate returning.\n");
#if Logs
---- Enclosure ----guy@auspex.auspex.com (Guy Harris) (11/22/89)
>i've discovered a bug in the wp code on SPARC. actually, it's a bug in >Sun's SPARC C compiler that's exercised by wp. (Actually, it's a bug in the WP code, which happens not to prevent it from doing what you want on many C implementations, but the SPARC and perhaps Pyramid C implementations are not among them; passing an object of type "foo" to a routine expecting an object of a union type that includes type "foo" is not valid C.) Is it ever the case that somebody passes something to "wp_DeAllocate" and doesn't know which of the N flavors of things that "wp_DeAllocate" can deallocate it is? If not, perhaps the best long-term fix is to replace "wp_DeAllocate" with multiple deallocaters and call the correct one in each instance, passing it a pointer to a "wp_PrimeKeySet" or to a "struct wp_SrchToken". (I.e., does all the code that calls it passing it a "struct wp_SrchToken *" really truly pass it a pointer to a "wp_SrchToken", not a pointer to a "wp_PrimeKeySet" in disguise, and *vice versa*?)
salzman%iris@RAND.ORG (Isaac Salzman) (11/22/89)
>(Actually, it's a bug in the WP code, which happens not to prevent it >from doing what you want on many C implementations, but the SPARC and >perhaps Pyramid C implementations are not among them; passing an object >of type "foo" to a routine expecting an object of a union type that >includes type "foo" is not valid C.) i didn't think it was valid C. defeinitely not code that i would write :-) (especially declaring the union as part of the paramater dec's - yech)! but since it worked under GCC on SPARC and both Sun's CC and GCC on the sun3.... >Is it ever the case that somebody passes something to "wp_DeAllocate" >and doesn't know which of the N flavors of things that "wp_DeAllocate" >can deallocate it is? If not, perhaps the best long-term fix is to >replace "wp_DeAllocate" with multiple deallocaters and call the correct >one in each instance, passing it a pointer to a "wp_PrimeKeySet" or to a >"struct wp_SrchToken". (I.e., does all the code that calls it passing >it a "struct wp_SrchToken *" really truly pass it a pointer to a >"wp_SrchToken", not a pointer to a "wp_PrimeKeySet" in disguise, and >*vice versa*?) i haven't explored the andrew code a whole lot. i do know that andrew uses (in some places) a classing mechanism. i guess it does not implement any sort of dynamic binding (aka virtual functions in C++). here's a good case for using an object system with dynamic binding. you wouldn't need to know which of the N flavors of an object something is. it's determined at runtime. just send the DeAllocate message (actually, just "delete" the object in C++ and the destructors get called automatically).... -isaac (salzman@rand.org)
bader+@ANDREW.CMU.EDU (Miles Bader) (11/22/89)
Isaac Salzman <salzman%iris@rand.org> writes: > i haven't explored the andrew code a whole lot. i do know that andrew uses > (in some places) a classing mechanism. i guess it does not implement any > sort of dynamic binding (aka virtual functions in C++). here's a good case > for using an object system with dynamic binding. you wouldn't need to know > which of the N flavors of an object something is. it's determined at > runtime. just send the DeAllocate message (actually, just "delete" the object > in C++ and the destructors get called automatically).... The "classing mechanism" used by atk ("class") implements ONLY dynamic binding of methods. However, the white pages doesn't use class. -Miles
cfe+@ANDREW.CMU.EDU ("Craig F. Everhart") (11/22/89)
I have to agree with Guy Harris that the correct thing to do would be to revoke the idea that wp_DeAllocate should take any of several different types. The only thing I can say in defense of the way that it currently works is that I wrote it long ago, when a relative novice to C and didn't understand the HC compiler's warnings. That being said, I've absorbed Isaac Salzman's change to flush the ``union'' from the parameter list to make the thing work moderately correctly, but it's a little late (tape going out this morning) to ``do the right thing'', implementing wp_DeAllocateSrch/wp_DeAllocatePKs, changing all the calls everywhere in all andrew code, and updating the documentation. That should be done, and maybe if we ever generate another version... Craig