0

sm is a 2D array of character pointers allocated dynamically. I need to understand why my pointer to pointer arithmetic is failing in conditional if in loop structure.

2nd column in sm is where the string is that I need to test with the grade key gk which is array of characters/string. s holds row size and q is column size for 2D array, hm is my heap memory counter for freeing function which is not importing for my question.

double *cals(char **sm, char *gk, int s, int q, unsigned *hm) {
    int c = 0;
    double *savg = malloc(s * sizeof(double));
    assert(savg);
    *hm += 1;

    for (int i = 0; i < s; *(savg + i) = c / q * 100 , c = 0,  ++i) {
        for (int j = 0; j < q; ++j) {
            if (*(*(sm + i * STUDENTATT + 1) + j) == *(gk + j)) 
                ++c;
        }
    }
    return savg;
}
Usman Maqbool
  • 3,351
  • 10
  • 31
  • 48
teaNcode
  • 19
  • 1
  • 7
  • Show the declaration of the variable passed as `sm`. In fact, please provide [**A Minimal, Complete, and Verifiable Example (MCVE)**](http://stackoverflow.com/help/mcve). – David C. Rankin Nov 09 '18 at 04:40
  • 3
    This: `sm + i * STUDENTATT + 1` looks to be past the bounds of what was allocated to `sm`, assuming it was something like `sm = malloc(s * sizeof(char *));` – dbush Nov 09 '18 at 04:42
  • yes it was, char ** sm = malloc(s * STUDENTATT *sizeof(char * )); is the heap allocated statement I used. STUDENTATT is a macro I defined which as a value of 2. so its just a s * 2 two dimensional array. – teaNcode Nov 09 '18 at 04:49
  • 1) You allocated pointers, but did you allocate actual storage for the `char`s? 2) `(sm + i * STUDENTATT + 1) + j` smells wrong, I am quite certain it should be `*(sm + i * q + j)`, if `q` is the number of columns (ditch the define, remove `+1`). 3) String comparison is done using `strcmp`, unless you are certain both pointers point to the same string. 4) You are don't something weird with `hm` - if that's you allocation/free debug counter, it might make more sense to create your own `MALLOC`/`FREE` macros and place the counter there, instead of passing it around. – vgru Nov 09 '18 at 09:07
  • This code is quite obfuscated. You should use the `[]` operator instead of `*(...)` and you need to drop the assignment of unrelated things from the 3rd clause of the for loop. Instead you should have something like `for(int i=0; i – Lundin Nov 09 '18 at 11:49
  • @Groo. WOW I never thought of having a macro as my counter for keeping track of heap allocation. that is brilliant. I will be trying that out next time. Ok I will try *(sm + i * q + j) and comment later about it.. I also am going to post this whole program because for some weird reason my printf() in my print function for this very same program is malfunctioned and telling me a read access violation has occurred. unclear as to the reason. – teaNcode Nov 10 '18 at 03:07

1 Answers1

0

There isn't much information given about the purpose of cals function so I had to make a few assumptions to write this answer.

Assumption-1(meaningful):- You want to find how much characters in the two strings are equal(no every characters) and then find the percentage of the same characters to the total characters. If that is the case use the below code.

double *cals(char **sm, char *gk, int s, int q, unsigned *hm) {
    float c = 0;        // To force float division the c is declared as a float variable
    double *savg = malloc(s * sizeof(double));
    assert(savg);
    *hm += 1;       
    char* sm_i_key = NULL;
    unsigned int strlen_gk = strlen(gk);    
    unsigned int key_length = string_gk;

    for (int i=0; i<s; ++i) { //The calculation is moved inside for loop
            sm_i_key = *(sm+i*q+1); // You can also use sm_i_key = &sm[i*q+1]
            /* Uncomment this section if length of 2 strings are not bound to be equal
            if(strlen(sm_i_key) < strlen_gk){
                    key_length = sm_i_key; 
            }
            else{
                    key_length = strlen_gk
            }
            */
            for (int j = 0; j < key_length; ++j) {
                    if (sm_i_key[j] == gk[j]) 
                        ++c;
            }
            savg [i] = c / strlen_gk * 100; /* Since gk is the grade key it is assumed
            to be equal to the total number.*/
            c = 0;
        }          
    return savg;
}

Assumption-2:- You want to check whether the strings whose starting address is stored in the second column of each row of a 2D array sm is equal to the string stored in array pointed by gk and then calculate a value(double). The function cals only returns 0.0 or 100.0 as the formula avgs[i]=c / q * 100 will only produce 0 if stings are not equal(since integer division c/q will always result in 0 if c is less than q which is the case here) and 100 if strings are equal(Then why use a double to store the value if only 0 and 100 is stored).

If that is the case then what you are doing here is fine unless the array gk and array sm[i][2] have different string length(not q). It would be better to use strncmp to check the equality of string if the string length of two array's are bound to be different. Use the below code to do that:-

double *cals(char **sm, char *gk, int s, int q, unsigned *hm) {
    int c;
    char* sm_i_key = NULL;
    double *savg = malloc(s * sizeof(double));
    assert(savg);
    *hm += 1;       
    for (int i=0; i < s;++i){//The calculation is moved to a static assignment given below
        if(strncmp(sm_i_key, gk, strlen(gk) == 0)
        {
            savg[i] = 100.0;    // Since c/q * 100 => 100.0 if q == c
        }
        else
        {
            savg[i] = 0.0;      /*Since c/q *100 => 0.0 if q < c since integer 
            division will result in 0.*/
        }
    }              
    return savg;
}

I hope it helps.

jibin_raj24
  • 84
  • 8