C N00B

[markfw]

Hi
I have some questions:
I'm getting segmentation fault on print_clauses(...) function and I really can't figure it out why.

But the bigger question is, am I doing something wrong in build_clauses(...) function? Am I using double pointers in right way? because the second #if ... #endif gives me N = 0 which it suppose to be 18 since I have 36 lines in the file and each line has 4 bytes in it.

Thanks in advance.
Mark

#include <stdio.h>
#include <stdlib.h>

#define __DEBUG__



/* In each clause, we have an OR between each var(s) */
struct __clause__
{
    /* 
     * INDEX 0 -> ID of the node -> X1, X2, ... -> 1, 2, ... is an ID
     * INDEX 1 -> 0 -> Not Negation, 1 -> Negation
     */
    int _var1[2];
    int _var2[2];
};


/* Function Prototype */
int  build_clauses(const char *, struct __clause__ **);
void print_clauses(const struct __clause__ *);



int 
main(int argc, char **argv)
{
    struct __clause__ *clauses;
    
    build_clauses("max2sat_data", &clauses);
    print_clauses(clauses);
    
    return 0;
} /* End Main */


int
build_clauses(const char *p_fname, struct __clause__ **p_clauses)
{
    char buffer[8]; /* (ID, SPACE, NEG, NEWLINE) * 2 -> 8-bytes */
    FILE *p_file;
    int size, num_x, i;
    
    p_file = fopen(p_fname, "rb");
    if (p_file == NULL) return -1;
    
    fseek(p_file, 0, SEEK_END);
    size = ftell(p_file);
    if (size % 2 != 0) ++size;   /* If size is not even add one byte to it */
    fseek(p_file, 0L, SEEK_SET); /* rewind */

#ifdef __DEBUG__
    printf("SIZE = %d, N = %d\n", size, size / 8);
#endif
    
    
    /* Reading file and build clauses */
    *p_clauses = 
        (struct __clause__ *) malloc(sizeof(struct __clause__) * (size / 8));
    if (*p_clauses == NULL) return -1;
        
    i = 0;
    while (!feof(p_file) && i < (size / 8))
    {
        fgets(buffer, 8, p_file);
        
        (*p_clauses + i)->_var1[0] = buffer[0] - '0';
        (*p_clauses + i)->_var1[1] = buffer[2] - '0';
        
        (*p_clauses + i)->_var2[0] = buffer[4] - '0';
        (*p_clauses + i)->_var2[1] = buffer[6] - '0';
        
        ++i;
    } /* End WHILE */
    
    *p_clauses -= (i - 1);

#ifdef __DEBUG__
    size = sizeof(*p_clauses) / sizeof(struct __clause__);
    printf("N = %d\n", size);
#endif
    
    fclose(p_file);
    return 0;
}

void
print_clauses(const struct __clause__ *p_clauses)
{
    int size, i;
    
    size = sizeof(p_clauses) / sizeof(struct __clause__);
    for (i = 0; i < 18; ++i);
    {
        if (i == 0)
            printf("(X%d V X%d)", (p_clauses + i)->_var1[0], p_clauses->_var2[0]);
        else
            printf(" ^ (X%d V X%d)", (p_clauses + i)->_var1[0], p_clauses->_var2[0]);
    } /* End FOR */
    
    printf("\n");
}

[nezachem]

If you mean lines 79-82, the logic is as follows: p_clause is declared as struct __clause ** . *p_clause is struct __clause__ * , that is, a pointer. sizeof(*p_clause) yields the size of pointer (most likely, 4), regardless of how much memory you did allocate.
Do realize that (1) sizeof is calculated at compile time, and (2) there's now way to determine a size of object pointed to by inspecting the pointer.

Regarding a segfault, line 77 screws you up. The intention was to restore the *p_clause to its original value after it's been modified by the loop, right? But the loop doesn't modify it.

[markfw]

nezachem, thanks for you great reply. Now I understand sizeof() more. Now another question is after I fixed the code (basically remove line 77) everything works except I get only one output.

My question is on line 59 and 60, am I doing it right? I think I do as you said *p_clauses is struct __clause__ *.

Thanks
Mark

[nezachem]

Basically, right. It is more idiomatic to use calloc here. Also, I prefer to set everything up as

struct __clause__ * build_clauses(const char * p_fname)
{
    struct __clause__ * clauses;
    ...
    clauses = calloc(..., ...);
    ....
    return clauses;
}

which removes one level of indirection and makes code more streamlined, but I can't see anything fundamentally wrong in your code.

Why you are not getting the desired output, I can't say. Time to run the proram in the debugger, step by step.

PS: the way you are filling up the array is very suspicious.

[abhimanipal]

Can you look the C operator precendence tables for the operators *(dereference operator) and +(addition operator) ?
Then look at the while loop you have written

while (!feof(p_file) && i < (size / 8))
    {
        fgets(buffer, 8, p_file);
        
        (*p_clauses + i)->_var1[0] = buffer[0] - '0';    
        (*p_clauses + i)->_var1[1] = buffer[2] - '0';
        
        (*p_clauses + i)->_var2[0] = buffer[4] - '0';
        (*p_clauses + i)->_var2[1] = buffer[6] - '0';
        
        ++i;
    } /* End WHILE */

[markfw]

abhimanipal, thanks for your reply.
I checked it since p_clauses is a type struct __clause__ ** (Double pointer), then *p_clauses is a type of struct __clause__ *.
So (*p_clauses + i) is should be the same as to say *p_clsuses (Please correct me if I'm wrong)

Thanks
Mark

[abhimanipal]

*p_clsuses will resolve to **(p_clsuses+i)

In your build clauses function ... You are passing the address of a pointer as an argument .... Is there any specifc reason for passing the adress ? Can you not pass the pointer directly ....

[markfw]

What I was trying to do is to create an array in build_clauses function and the return of that function indicate if it was successful or not. Thats why I passes the address of the pointer to the function in order to create the array and put values in it.

Mark

[abhimanipal]

Ok ... I get it now ...

Whats the problem now ?

[markfw]

Now the problem is print_clauses(...) prints X0, X0 which I dont even have that on the file. Plus it only prints one line instead of 18 lines (18 is a number of clauses I have.

Do you know why it is only printing one line instead of 18?

Mark

[thomas_naveen]

Please check the following.

1) Your file "max2sat_data" is present in the current directory with read permissions.
2) What is build_clauses returning? Is memory allocated to clauses when you are calling print_clauses?

[markfw]

Hi sorry for late reply.
Yes the fine (max2sat_data) is in same directory as the program itself and it does have a read and write permission.

The return from build_clauses is 0 (Successful)
The output from print_clauses is
(X0 v X0) ^

Any idea why?

--
Thanks
Mark

[abhimanipal]

You have a hard coded for loop which will run 18 times.
What does it print the rest of time ?

[markfw]

Thanks for your reply.
No but if you look at the DATA file that I have, that array should have 18 elements in there.
Also the memory address for the array after build_clauses function executed is: 0x100100080, The return from that function is 0

Data file:

1 0
2 1
2 0
3 0
1 1
3 0
2 1
4 1
1 1
4 0
3 1
4 0
1 1
2 1
1 0
3 0
1 1
4 1
2 0
3 1
2 0
4 0
3 1
4 1
1 1
2 0
1 0
3 1
1 0
1 0
2 1
2 1
3 0
3 0
4 1
4 1

Thanks
Mark

[thomas_naveen]

Please remove this ";" at the end of the for loop in print_clauses.

for (i = 0; i < 18; ++i);

[markfw]

thomas_naveen,
Thanks for your help, I'm getting output. Thank you. Now I feel that I'm stupid, lol.

Thanks again for the help.

--
Mark