edh@ux.acs.umn.edu (Eric D. Hendrickson) (11/30/90)
The below program does not work. It seg faults in the last for loop. Can
someone see what is wrong here? I suspect I have made a mistake using the
pointers somewhere, but after much trial and error, little progress has
been made. Perhaps you can see what it is...
thanks,
Eric
(btw -- if this is not a good question for comp.lang.c, please let me know)
/*
* PrintCap Utility (skeleton test version)
* Copyright 1990
* placed in the public domain
* Eric Hendrickson <{edh,eric}@ux.acs.umn.edu>
* November, 1990
*/
#define MAXPRINTERS 100 /* Max of 100 printers */
#define PRINTCAP "/etc/printcap"
#define PROGRAM "pcu"
#include <stdio.h>
#include <sys/types.h>
#include <string.h>
main()
{
char *grep = "cm";
chores(grep);
}
/* extract the printcap setting requested */
char **
extract(grep)
char grep[];
{
FILE *fp;
int i = 0, j = 0;
char found[MAXPRINTERS][BUFSIZ]; /* holds found entries */
char line[BUFSIZ]; /* holds search string */
char *p, *pc = PRINTCAP;
if ((fp = fopen(pc, "r")) != NULL) {
while (fgets(line, sizeof(line), fp)) {
if (line[0] == '\t') { /* we have a line to scan */
if (p = (char *)strstr(line, grep)) { /* we have a candidate */
if ((i = strcspn(p, (char *)"=#")) != 2) { /* guess not */
continue;
}
p+=3; /* skip the prefix */
strtok(p, (char *)":"); /* chop off the trailing section */
strncpy((char *)found[j++], p, strlen(p));
}
}
}
fclose(fp);
return((char **)found);
} else {
fprintf(stderr, "%s: unable to open %s\n", PROGRAM, PRINTCAP);
exit(1);
}
}
/*
* perform requested hyphen options
*/
int
chores(grep)
char *grep;
{
static char **gots;
char **extract();
gots = (char **)extract(grep);
for( ; **gots != (char)'\0'; printf("%s\n", gots++)) ;
}
/*
* below are two seperate functions from comp.sources.unix, I keep them
* in seperate from the main program.
*/
/*
* strstr - find first occurrence of wanted in s
*/
#define NULL 0
char * /* found string, or NULL if none */
strstr(s, wanted)
char *s;
char *wanted;
{
register char *scan;
register int len;
register char firstc;
extern int strcmp();
extern int strlen();
/*
* The odd placement of the two tests is so "" is findable.
* Also, we inline the first char for speed.
* The ++ on scan has been moved down for optimization.
*/
firstc = *wanted;
len = strlen(wanted);
for (scan = s; *scan != firstc || strncmp(scan, wanted, len) != 0; )
if (*scan++ == '\0')
return(NULL);
return(scan);
}
/*
* strcspn - find length of initial segment of s consisting entirely
* of characters not from reject
*/
int
strcspn(s, reject)
char *s;
char *reject;
{
register char *scan;
register char *rscan;
register int count;
count = 0;
for (scan = s; *scan != '\0'; scan++) {
for (rscan = reject; *rscan != '\0';) /* ++ moved down. */
if (*scan == *rscan++)
return(count);
count++;
}
return(count);
}
--
/----------"Oh carrots are divine, you get a dozen for dime, its maaaagic."--
|Eric (the "Mentat-Philosopher") Hendrickson Academic Computing Services
|edh@ux.acs.umn.edu The game is afoot! University of Minnesota
\-"What does 'masochist' and 'amnesia' mean? Beats me, I don't remember."--gwyn@smoke.brl.mil (Doug Gwyn) (11/30/90)
In article <2784@ux.acs.umn.edu> edh@ux.acs.umn.edu (Eric D. Hendrickson) writes: >(btw -- if this is not a good question for comp.lang.c, please let me know) It probably isn't a good idea to post a large chunk of source code and ask the net to debug it for you.. > if (p = (char *)strstr(line, grep)) { /* we have a candidate */ This cast bothers me. If you put it there in an attempt to compensate for strstr() not being declared in <string.h>, this isn't the right way to do it. Instead of allowing strstr() to be implicitly declared as returning int and trying to fix it by applying a cast, you should declare the function properly before using it: extern char *strstr(); > return((char **)found); Now here is probably the heart of your bug. You should view all casts with suspicion, since they indicate that you're trying to apply force to an object to make it be something that is against its nature. `found' is not actually a pointer to a pointer; it's an array of arrays -- which is NOT AT ALL the same thing. (I think Chris Torek has a standard dissertation on this that he posts from time to time.) > gots = (char **)extract(grep); > for( ; **gots != (char)'\0'; printf("%s\n", gots++)) ; And now the bug bites. `gots' starts out pointing to found[0][0], but with a weird type. `*gots' would attempt to pick up a char* from that address (for an operand of a second *), and of course there is no char* datum stored there. The other uses of `gots' simply add confusion, and eventually something breaks from all the use of invalid pointers to fetch stuff.
henry@zoo.toronto.edu (Henry Spencer) (12/01/90)
In article <14619@smoke.brl.mil> gwyn@smoke.brl.mil (Doug Gwyn) writes: >>(btw -- if this is not a good question for comp.lang.c, please let me know) > >It probably isn't a good idea to post a large chunk of source code and ask >the net to debug it for you.. Especially since a good many of us deliberately ignore such postings. If you can't be bothered pinning down where in your program the trouble lies, I'm not going to do it for you. -- "The average pointer, statistically, |Henry Spencer at U of Toronto Zoology points somewhere in X." -Hugh Redelmeier| henry@zoo.toronto.edu utzoo!henry
rmartin@clear.com (Bob Martin) (12/02/90)
In article <2784@ux.acs.umn.edu> edh@ux.acs.umn.edu (Eric D. Hendrickson) writes: >The below program does not work. It seg faults in the last for loop. Can >someone see what is wrong here? I suspect I have made a mistake using the >pointers somewhere, but after much trial and error, little progress has >been made. Perhaps you can see what it is... > >thanks, > Eric >(btw -- if this is not a good question for comp.lang.c, please let me know) ^^^^^^^^^^^ I can't think of a better place.^^^^^^^^^^^ ======== WARNING WILL ROBINSON, EXTREME SOURCE CODE EDITTING.======= >/* extract the printcap setting requested */ >char ** >extract(grep) >char grep[]; >{ > char found[MAXPRINTERS][BUFSIZ]; /* holds found entries */ > strncpy((char *)found[j++], p, strlen(p)); > return((char **)found); >} > >int >chores(grep) >char *grep; >{ > static char **gots; > char **extract(); > > gots = (char **)extract(grep); > for( ; **gots != (char)'\0'; printf("%s\n", gots++)) ; >} > The crux of the problem is twofold. 1) the 'found' array is not static, and so won't exist when 'chores' want to print it. 2) the 'found' array cannot properly be cast to a char**. The reasons why this is true have been discussed profusely on the net in recent weeks and so I will not reiterate them here. -- +-Robert C. Martin-----+:RRR:::CCC:M:::::M:| Nobody is responsible for | | rmartin@clear.com |:R::R:C::::M:M:M:M:| my words but me. I want | | uunet!clrcom!rmartin |:RRR::C::::M::M::M:| all the credit, and all | +----------------------+:R::R::CCC:M:::::M:| the blame. So there. |
dcm@moria.UUCP (David C. Miller) (12/07/90)
In article <2784@ux.acs.umn.edu> edh@ux.acs.umn.edu (Eric D. Hendrickson) writes: >The below program does not work. It seg faults in the last for loop. Can >someone see what is wrong here? I suspect I have made a mistake using the >pointers somewhere, but after much trial and error, little progress has >been made. Perhaps you can see what it is... > >thanks, > Eric >(btw -- if this is not a good question for comp.lang.c, please let me know) >char ** >extract(grep) >char grep[]; >{ ... > char found[MAXPRINTERS][BUFSIZ]; /* holds found entries */ ... > return((char **)found); ... >} ... >int >chores(grep) >char *grep; >{ > static char **gots; > char **extract(); > > gots = (char **)extract(grep); > for( ; **gots != (char)'\0'; printf("%s\n", gots++)) ; >} You have 2 major problems: 1. In extract() you are returning a pointer to an automatic variable. Once you return from extract() found[] no longer exists and references to its address yeild undefined results. 2. Also, found[] is not initialized. Automatic variables are not automatically initialized to zeros. chores() expects to find zeros in the first unused slot in found[]. Fortunately, the solution is quite easy. Move found[] outside the extract() function and make it static. Moving it out of the function eliminates both problems, making it static makes it invisible to functions outside of this file. However, if you intend to call extract() more than once, you'll have to make the following change to extract(): *** Before --- After *** 48,49 fclose(fp); return((char **)found); --- 48,50 fclose(fp); + found[j][0] = '\0'; return((char **)found); Laters, David
chris@mimsy.umd.edu (Chris Torek) (12/08/90)
>In article <2784@ux.acs.umn.edu> edh@ux.acs.umn.edu (Eric D. Hendrickson) posts a large lump of code and asks why it crashes. In article <386@moria.UUCP> dcm@moria.UUCP (David C. Miller) writes: >You have 2 major problems: > 1. In extract() you are returning a pointer to an automatic variable. ... > 2. Also, found[] is not initialized. ... Doug Gwyn has pointed out a more fundamental error than either of these, namely, the type of the object being `returned' from extract and the type of the array `found' are entirely different. I doubt I will ever say this too often: You cannot, in C (as in many other languages), decide what a program fragment does until you both (A) know the types of all the expressions in that fragment are and (B) can show that they are mixed correctly. (If (B) fails, but you wrote the compiler or have equivalent knowledge, you can predict the machine's state anyway. This is useful for writing device drivers, but not for portable code.) Since the original program contains fundamental data-typing errors, it is impossible to say what it does. Once those are fixed (mentally if not actually), the two problems David Miller describes show up. -- In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (+1 301 454 7163) Domain: chris@cs.umd.edu Path: uunet!mimsy!chris