yxz1684@hertz.njit.edu (Ying Zhu ee) (03/01/91)
The following is a small program from "Mastering C pointers" by Robert J.
Traister. I am confusing with the pointer here. Since the r[100] is an
automatic variable, after calling the "combine" the r[100] will be deallocated.
Then the p will point to an unsafe place. Actually, runing this program
on VAXII/GPX(ULTRIX) and Sparc( BSD UNIX ) here have different answers. Any
one familiar with C can help me? Thank you!
main()
{
char a[10], b[10], *p, *combine();
strcpy( a, "horse" );
strcpy( b, "fly" );
p=combine(a, b);
printf("%s\n", p );
}
char *combine(s,t)
char *s, *t;
{
int x,y;
char r[100];
strcpy(r,s);
y=strlen(r);
for ( x=y; *t != '\0'; ++x )
r[x] = *t++;
r[x]='\0';
return(r);
}gordon@osiris.cso.uiuc.edu (John Gordon) (03/02/91)
yxz1684@hertz.njit.edu (Ying Zhu ee) writes: >main() >{ > char a[10], b[10], *p, *combine(); > strcpy( a, "horse" ); > strcpy( b, "fly" ); > p=combine(a, b); ^^^^^^^^^^^^^^^^ -> This is wrong. 'p' has not yet been correctly initialized. pointers need to be initialized before you can set them equal to something. You could change p's declaration to char p[256] and then do a strcpy(p, combine(a, b)); . --- John Gordon Internet: gordon@osiris.cso.uiuc.edu #include <disclaimer.h> gordon@cerl.cecer.army.mil #include <clever_saying.h>
torek@elf.ee.lbl.gov (Chris Torek) (03/02/91)
In article <2488@njitgw.njit.edu> yxz1684@hertz.njit.edu (Ying Zhu ee) writes: >The following is a small program from "Mastering C pointers" by Robert J. >Traister. [example deleted] ... Since the r[100] is an automatic variable, >after calling the "combine" the r[100] will be deallocated [and] p will >point to an unsafe place. Correct. If the example in this book appears exactly as shown in <2488@njitgw.njit.edu>, I would suggest either discarding the book entirely (reselling it merely pushes the bogus information on someone else) or---if this is atypical, and merely one mistake out of a great deal of correct code---advising the author and/or publisher, and marking the example in the book to note its error or repair it. (One `static' will repair this particular sample, for instance.) Note that the only excuse these days for buggy examples is when the publisher do their own re-typesetting, since book examples should be typeset by direct insertion of working code into the typesetting system. There are many C books available these days, and a large fraction of them are inexcusably bad. Do not trust any single source (except perhaps a copy of the ANSI C standard X3.159-1989). -- In-Real-Life: Chris Torek, Lawrence Berkeley Lab EE div (+1 415 486 5427) Berkeley, CA Domain: torek@ee.lbl.gov
scs@adam.mit.edu (Steve Summit) (03/02/91)
In article <2488@njitgw.njit.edu> yxz1684@hertz.njit.edu (Ying Zhu ee) writes: >The following is a small program from "Mastering C pointers" by Robert J. >Traister. I am confusing with the pointer here. Since the r[100] is an >automatic variable, after calling the "combine" the r[100] will be deallocated. >Then the p will point to an unsafe place. You are the innocent victim of a cruel hoax. You assumed that a book entitled "Mastering C Pointers" might have been written by someone who was, himself, a master in the use of C pointers. Evidently this is anything but the case. You would be well advised to throw this book very far away; clearly you already know more about correct pointer use than does its author. I can't quite believe that the code sample presented was actually published, but my friendly neighborhood technical bookstore has closed for the evening, so I'll have to take the complainant's word for it. In fact, returning a pointer to auto storage which is no longer valid is only the worst of the code's several problems. Let's take another look at it, shall we? (Put your rubber gloves on first, and preferably a clothespin on your nose, too.) >main() >{ > char a[10], b[10], *p, *combine(); > > strcpy( a, "horse" ); > strcpy( b, "fly" ); > > p=combine(a, b); > > printf("%s\n", p ); >} The strings a and b are initialized rather oddly, and main() neither returns a value nor calls exit(), but these are minor. The "combine" routine is far more interesting: >char *combine(s,t) >char *s, *t; >{ > int x,y; > char r[100]; > > strcpy(r,s); > y=strlen(r); > for ( x=y; *t != '\0'; ++x ) > r[x] = *t++; > r[x]='\0'; > > return(r); >} First of all, the returned result array r is, as mentioned, an automatic array, a pointer to which should not be returned. The array's size is fixed, and would be exceeded by easily imaginable calling patterns. Overflow is not checked. None of the variables have very good names. A variable named "x" is used as an index; this is somewhat unusual, especially since this is an algorithm (copying characters hither and yon) which no C programmer worthy of the title would code using an array subscript. A pointer solution is far more likely, especially in a book entitled "Mastering C Pointers." (Perhaps a second example in the book showed how to convert the routine to use all pointers.) A variable named "y" is used once, as the length of the first of the two strings to be combined; it is unneded, as the call to strlen might as well appear in the loop initialization expression. It is unfortunate that the first string has to be traversed a second time (by the call to strlen) to determine its length, when that could have been determined as a side effect of the initial copy, but the side effect is buried inside strcpy. The use of standard library routines to do part of a job is generally commendable, though (as long as the job is worth doing -- see below), so we'll let this point pass. We could rewrite the routine taking the above observations into account, but wait a minute. The middle four lines of the "combine" routine could be replaced with a call to the standard library routine strcat. Clearly, calling upon standard library routines is not considered objectionable in this code, as strcpy and strlen have been used already. Before rewriting "combine" to use only strcpy and strcat, however, we might note that "combine" is almost identical to strcat, except that strcat appends the second string to the first and returns its first argument, while "combine" tries to build a third string. But that's the part it gets most wrong. Rather than hauling in this useless "combine" routine, the author would have been much better advised to demonstrate simple implementations of the standard library routines such as strlen, strcpy, and strcat. (It would be wise to call them my_strlen, my_strcpy, and my_strcat, so that the student would not abuse a reserved identifier when compiling his own version, and to permit regression testing.) I have often though that it would be wonderful if Kernighan and Plaugher could come out with a third edition of their classic Elements of Programming Style, incorporating C examples instead of PL/I and Fortran. I thought that perhaps there were not enough published bad examples to serve as the base for such a book. Silly me. Mastering C Pointers would clearly be a mother lode. Whoops, someone else just tried to answer the original question. Let's take a look: In article <1991Mar1.235912.25133@ux1.cso.uiuc.edu> gordon@osiris.cso.uiuc.edu (John Gordon) writes: >> p=combine(a, b); > ^^^^^^^^^^^^^^^^ -> This is wrong. 'p' has not yet been correctly > initialized. pointers need to be initialized > before you can set them equal to something. Mr. Gordon needs to learn more about pointers and initialization. The only thing wrong about p's initialization here is that the pointer value that "combine" returns is bogus; this is combine's fault, not p's. Yes, pointers need to be initialized before they can be used, but what better way to initialize one than by sticking it on the left hand side of an equals sign and putting an expression of type "pointer to char" on the right? The suggestion that we > could change p's declaration to char p[256] and > then do a strcpy(p, combine(a, b)); . wouldn't hurt, although it would make the identifier "p" an array, not a pointer (see the FAQ!), which is a bit misleading. (It's also silly, in this case, since "combine" was trying to return a pointer to valid storage, although no more silly than the original code's declaring a[] and b[] and strcpying into them.) Steve Summit scs@adam.mit.edu
art@pilikia.pegasus.com (Art Neilson) (03/03/91)
In article <2488@njitgw.njit.edu> yxz1684@hertz.njit.edu (Ying Zhu ee) writes: >The following is a small program from "Mastering C pointers" by Robert J. >Traister. I am confusing with the pointer here. Since the r[100] is an >automatic variable, after calling the "combine" the r[100] will be deallocated. >Then the p will point to an unsafe place. Actually, runing this program >on VAXII/GPX(ULTRIX) and Sparc( BSD UNIX ) here have different answers. Any >one familiar with C can help me? Thank you! You are right, it is unsafe as storage for automatic r within function combine is released upon return to main. >main() >{ > char a[10], b[10], *p, *combine(); > > strcpy( a, "horse" ); > strcpy( b, "fly" ); > > p=combine(a, b); Why not use strcat(3) instead of the combine() function ? (of course you'd have to declare a[20]) p=strcat(a, b); I guess it'd defeat the purpose of the excersize. > printf("%s\n", p ); >} > >char *combine(s,t) >char *s, *t; >{ > int x,y; > char r[100]; > > strcpy(r,s); > y=strlen(r); > for ( x=y; *t != '\0'; ++x ) > r[x] = *t++; > r[x]='\0'; > > return(r); >} -- Arthur W. Neilson III | INET: art@pilikia.pegasus.com Bank of Hawaii Tech Support | UUCP: uunet!ucsd!nosc!pilikia!art