Re: wierd behavior in program

newbiegalore wrote, On 23/02/08 06:12:

<snip>

below), the second time this function is called and an assignment made
to cstack[1]=line (line is /head), it seems that cstack[0] also
becomes /head, whereas it should remain /html. I checked using GDB,
that as soon as control enters this function for the second time, when
head has been read and /head has been constructed, but has not yet
been assigned to cstack[1], even then cstack[0] equals /head!

If someone spots an obvious pointer assignment error please help me
out. Some idea about how to proceed will be great.

<snip>

In C there is no magic assignment operator for strings, so if you want to copy strings about you should look up strcpy.

#include<stdlib.h>

They don't charge for spaces and a space would make this more readable.
#include <stdlib.h>

#include<stdio.h>
#include <string.h>
#define MAXTAGLEN 40
#define MAXDEGREE 500
#define MAXSDEPTH 2500

struct node {
int degree;
char* type;
struct node* next[MAXDEGREE];
struct node* prev;
};

typedef struct node tag;

void initstack(char* cstack[]){
int a;

for(a=0;a<MAXSDEPTH;a++){
cstack[a]=(char*)malloc(sizeof(char)*MAXTAGLEN);

Loose the cast, it isn't needed, makes the code harder to read and on some implementations will hide the warning/error if you fail to include stdlib.h
cstack[a] = malloc(sizeof(char)*MAXTAGLEN);
Then be aware that this relies on you keeping types in sink. You can reduce maintenance work by doing
cstack[a] = malloc(MAXTAGLEN * sizeof *cstack[a]);
See, you don't need to look up to check the type is correct or change this line if the type changes. Then, of course, since this is a string and likely to always be one, and char is 1 byte by definition in C we can drop the sizeof completely.
cstack[a] = malloc(MAXTAGLEN);
Far simpler.

If you are always going to have a fixed size you might as well have an array in the structure rather than a pointer. If you are going to use a pointer you might as well initialise it to NULL and then only allocate the space when you know how much you need and you need it.

*cstack[a]=(char)a;

Why the cast? It does you no good. It also does not magically create a string and I'm not sure what you are trying to achieve by it.

printf("init %s\n", cstack[a]);

Given that you haven set up a string

}
}

void makeendtag(char* tag, char* endtag){
char *tmp;

tmp=(char*)malloc(sizeof(char)*MAXTAGLEN);
memmove (tmp+1,tag,strlen(tag));

This won't copy the nul termination so it won't be a string

*(tmp+0)='/';
strcpy(endtag,tmp);

Making this strcpy invalid.

In any case, you could have avoided the extra allocation by simply doing
endtag[0] = '/';
strcpy(endtag+1,tag);

free(tmp);
}

int checktop(int depth, char* cstack[], char* tag){
int match;

printf("matching %s and %s stack depth %d\n", cstack[depth], tag,
depth);
match=strcmp(cstack[depth], tag);
return(match);
}

void pushtag(int d, char* line, char* cstack[], tag* pointer){
int
a; //
whats going on?

Please don't use stupidly long lines and don't use // style comments when posting to news groups. It meant that rather than being able to copy/paste and then compile you code I first had to fix the line-wrapping which made the program invalid.

cstack[d]=line;

This does not copy the contents of the line array to cstack[d], rather it overwrites the pointer to your allocated space with a pointer to line. You know about strcpy as you used it earlier so why did you not use it here?

printf("line is %s cstack[%d] is %s \n", line, d, cstack[d]);
//tstack[d]=pointer;
for(a=0;a<=d;a++)
printf("Stack %d %s\n", a, cstack[a]);
}

void poptag(int depth, char* cstack[]){
printf("popstack depth %d\n", depth);
cstack[depth-1]="000000000000000";
//tstack[depth-1]=NULL;
}

tag* initheadtag(char* line){
int a;

tag* head=(tag*)malloc(sizeof(tag));

Again, it would be simpler to use
tag *head = malloc(sizeof *tag);
Note also the change to the spacing. This is because if you do
tag* head,foo;
foo will NOT be a pointer but a variable of type tag.

head->degree=0;
head->type=line;
for(a=0;a<MAXDEGREE;a++){
head->next[a]=NULL;
head->prev=NULL;
return(head);

You do not need the parenthesis
return head;

}
}

tag* inittag(char* line, int depth){
int a;

tag* curr=(tag*)malloc(sizeof(tag));

See previous comment.

curr->degree=0;
curr->type=line;
for(a=0;a<MAXDEGREE;a++){
curr->next[a]=NULL;
/*get top addr*/
//curr->prev=tstack[depth-1];
//curr->prev->next[curr->prev->degree]=curr;
//curr->prev->degree+=1;
return(curr);
}
}

int main(int argc, char **argv) {

FILE *fplong, *fpshort;
int a,b,d,end,lnum,match,lineslong,linesshort;
char *line, tmp[MAXTAGLEN];
char* cstack[MAXSDEPTH];
tag* tstack[MAXSDEPTH];

line=malloc(sizeof(char)*MAXTAGLEN);

Why bother making line malloc'd rather than a simple array?

for(a=0;a<MAXSDEPTH;a++)
tstack[a]=(tag*)malloc(sizeof(tag*));
/*check input*/
if(argc!=5){
printf("you have entered %d arguments\n", argc);
printf("USAGE: binary-name long-file numoflines-in-longfile short-
file numoflines-in-shortfile\n");
exit(0);
}

/*open files*/
if((fplong=fopen(argv[1],"r"))==NULL){
printf ("Error opening long data file\n");
exit(0);

It would be more conventional to return an error, i.e.
exit(EXIT_FAILURE);
Or
return EXIT_FAILURE;

}

if((fpshort=fopen(argv[3],"r"))==NULL){
printf ("Error opening short data file\n");
exit(0);
}
/*accept inputs*/
lineslong=atoi(argv[2]);
linesshort=atoi(argv[4]);

atoi is generally bad. Look up the strto functions which allow you to do some error checking.

/*start parsing*/

/*init stack*/
initstack(cstack);
d=0; //depth
end=0;
for(lnum=0;lnum<lineslong;lnum++){
/*read tag*/
fscanf(fplong, "%s", line);

<snip>

NEVER use fscanf like that. In the same vain, NEVER use gets. Either use fgets or use a length specifier if you use fscanf. Note that the scanf family is tricky to use correctly when you are inexperienced.

I've not checked the rest of your code.
--
Flash Gordon
.

Relevant Pages

  • Re: Memory Structure Pointer Problems
    ... typedef struct sta { ... char* name; ... int num_cmpnds; ... A pointer to a struct cmp is almost ...
    (comp.lang.c)
  • Re: Insufficient guarantees for null pointers?
    ... will the compiler know what the bounds are after converting that char * ... to an int *, if it could point to either of two arrays which happen to ... compares equal to the original pointer. ...
    (comp.std.c)
  • Re: problem with memcpy and pointers/arrays confusion - again
    ... int line, unsigned long *total_mem) ... That's a long pointer address... ... If sizeof > sizeof which is ... if you allocate for char with sizeof < sizeof, ...
    (comp.lang.c)
  • Re: Request critique of first program
    ... Returns a pointer to an asplit_result struct (unless unable to ... Success is indicated by SUCCESS, ... const char *out_file_b, ... const long int num_lines); ...
    (comp.lang.c)
  • Re: static in [ ]
    ... void someFunc (int size, char a) ... and passing a pointer to fewer than size characters is fine. ...
    (comp.lang.c)