wierd behavior in program

Discussion in 'C Programming' started by newbiegalore, Feb 23, 2008.

  1. newbiegalore

    newbiegalore Guest

    Hello everyone :),
    I am facing a small problem while coding
    up a relatively simple program. I am reading a file with html tags in
    it and am storing them in a character stack. The input file looks like
    html
    head
    /head
    /html

    I read each tag and push it on the stack (cstack) and pop it when I
    find the complimentary tag (/html for html and so on). I have declared
    the stack as char* cstack[MAXSDEPTH] and assign a value to this by
    cstack[a]=line , where line is a char* which holds the input. As I
    understand, cstack[a] points to a character string, and so I allocated
    some memory to hold the character string like html etc.. When I read
    in html, I create its complement, /html and push it in the stack, so
    that when I read /html and match it with the top-most element, I can
    pop it off the stack. In function pushtag, in my code (attached
    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.

    Some debug output from the program.

    read html
    made /html
    line is /html cstack[0] is /html
    Stack 0 /html
    read head
    matching /html and head stack depth 0
    line is /head cstack[1] is /head
    Stack 0 /head THIS IS THE ERROR!!! cstack[0] should be /
    html !!!
    Stack 1 /head
    read /head
    matching /head and /head stack depth 1
    popstack depth 2
    depth reduced to 1
    read /html
    matching /head and /html stack depth 0
    line is //html cstack[1] is //html
    Stack 0 //html
    Stack 1 //html
    ********************************************************************************************************
    the actual code.
    ********************************************************************************************************

    #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);
    *cstack[a]=(char)a;
    printf("init %s\n", cstack[a]);
    }
    }

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

    tmp=(char*)malloc(sizeof(char)*MAXTAGLEN);
    memmove (tmp+1,tag,strlen(tag));
    *(tmp+0)='/';
    strcpy(endtag,tmp);
    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?
    cstack[d]=line;
    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));
    head->degree=0;
    head->type=line;
    for(a=0;a<MAXDEGREE;a++){
    head->next[a]=NULL;
    head->prev=NULL;
    return(head);
    }
    }

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

    tag* curr=(tag*)malloc(sizeof(tag));
    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);
    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);
    }

    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]);

    /*start parsing*/

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

    /*is tag == type at top of stack with / at front*/

    /*match top of stack*/
    if(d>0){
    match=checktop(d-1, cstack, line);
    /*if match is true*/
    if(!match){
    /*pop stack*/
    poptag(d, cstack);
    /*reduce depth*/
    --d;
    printf("depth reduced to %d\n", d);
    /*check if depth 0*/
    if(d==0){
    printf("reached end\n");
    end=1;
    }
    }
    else{
    /*make the end tag*/
    makeendtag(line, tmp);
    /*push tag*/
    pushtag(d, tmp, cstack, inittag(line, d));
    /*increase depth*/
    ++d;

    }
    }
    else{
    /*make the end tag*/
    makeendtag(line, tmp);
    printf("made %s\n", tmp);
    /*push tag*/
    pushtag(d, tmp, cstack, initheadtag(line));
    /*increase depth*/
    ++d;
    }
    }
    return(0);
    }
     
    newbiegalore, Feb 23, 2008
    #1
    1. Advertising

  2. newbiegalore

    newbiegalore Guest

    So now it seems that the most recent tag I read, after I make the
    complement of that (/tag) , this value overwrites all elements of
    cstack, cstack[0], [1], [2] ... etc.
     
    newbiegalore, Feb 23, 2008
    #2
    1. Advertising

  3. newbiegalore

    newbiegalore Guest

    fixed i1..well not really, coz I used static arrays. If anyone knows
    what went wrong here, kindly let me know.

    Thanks,
    -A
     
    newbiegalore, Feb 23, 2008
    #3
  4. newbiegalore

    Flash Gordon Guest

    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
     
    Flash Gordon, Feb 23, 2008
    #4
  5. newbiegalore

    newbiegalore Guest

    wow thanks a ton for the detailed and extremely helpful
    commentary..super!!

    I'll make an effort to be a better coder.

    Thanks for your time,
    -A
     
    newbiegalore, Feb 23, 2008
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. paul reed

    Wierd Behavior of __doPostBack

    paul reed, Jul 8, 2003, in forum: ASP .Net
    Replies:
    1
    Views:
    582
    Bassel Tabbara [MSFT]
    Jul 8, 2003
  2. Steve

    Wierd datalist behavior

    Steve, Jul 8, 2003, in forum: ASP .Net
    Replies:
    0
    Views:
    346
    Steve
    Jul 8, 2003
  3. =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=

    Wierd behavior with text box postback

    =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=, Jun 21, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    576
    =?Utf-8?B?TWljaGFlbCBMb3VnaHJ5?=
    Jun 22, 2005
  4. Qun Cao

    wierd threading behavior

    Qun Cao, Oct 15, 2005, in forum: Python
    Replies:
    6
    Views:
    304
    dcrespo
    Oct 18, 2005
  5. subaruwrx88011

    Wierd Map Behavior

    subaruwrx88011, Feb 20, 2006, in forum: C++
    Replies:
    5
    Views:
    395
    subaruwrx88011
    Feb 20, 2006
Loading...

Share This Page