1

I am trying to write a solution for the USACO's Palindromic Squares After a lot of checks, although I found a lot of bugs, I still can't find why my program is still consisting on stopping. I believe it is a kind of a memory management problem, but I don't get why or how it is so. So, here is the code:

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


char ch(int x){
    if (x < 0 || x > 19) return 0;
    return "0123456789ABCDEFGHIJ"[x];
}

void append( int x, char* num){
    num=realloc(num,sizeof(char)* strlen(num)+2);
    num[strlen(num)+1] = '\0';
    num[strlen(num)] = ch(x);

}

char * baseB(int x, int base){
    int mult=1,lim=1,i;
    char *num;
    num = malloc(sizeof(char));
    num[0] = '\0';
    while(x/(mult*base)){
        mult*=base;
        lim++;
    }
    for(i=0;i<lim;i++){
        append(x/mult,num);
        x %= mult;
        mult/=base;
    }
    return num;
}

int is_pal( char* num ){
    int i;

    for(i=0;i<strlen(num)/2;i++){
        if ( num[i] != num[strlen(num)-1-i] )
            return 0;
    }
    return 1;
}
int main(){

    int x, size=0, y, base;
    int *lst;
    FILE *fp;
    lst=malloc(sizeof(int));
    fp= fopen("palsquare.in","r");
    fscanf(fp,"%d", &base);
    fclose(fp);

    for(x=1;x<301;x++){
        y = x*x;
                    printf(" a0 ");

        printf("%s ", baseB(y,base));
                    printf(" a1 ");

        //printf("%d ", is_pal( baseB(y,base) ) );
                    printf(" a2 ");

        if( is_pal( baseB(y,base) ) ){
            printf(" a3\n");
            size++;
            lst=realloc(lst,sizeof(int)*size);
            lst[size-1]=x;
        }
    }

    fp=fopen("palsquare.out","w");
    for(x=0;x<size;x++){
        fprintf(fp, "%d %d\n", lst[x], lst[x]*lst[x]);
    }
    fclose(fp);
    return 0;


}

The loop to create the result list, seemed to me as the reason of my problem. Any ideas about what happens there, why happens there?


edits:

  1. Changed the switch code :)
  2. Free'd all the calls to baseB
  3. lst is not a pointer anymore

the code to main() is now:

int main(){

    int x, size=0, y, base;
    int lst[300];
    FILE *fp;
    char *tmp = NULL;
    fp= fopen("palsquare.in","r");
    fscanf(fp,"%d", &base);
    fclose(fp);

    for(x=1;x<301;x++){
        y = x*x;
        tmp=baseB(y,base);
        printf("%s ", tmp);
        if( is_pal( tmp ) ){
            size++;
            lst[size-1]=x;
        }
        free(tmp);
        tmp=NULL;
    }

    fp=fopen("palsquare.out","w");
    for(x=0;x<size;x++){
        fprintf(fp, "%d %d\n", lst[x], lst[x]*lst[x]);
    }
    fclose(fp);
    return 0;


}
Ka Putsu
  • 27
  • 7
  • 9
    That `switch` is pure beauty. –  Oct 08 '13 at 17:23
  • 5
    [**Debug**](http://stackoverflow.com/questions/2590931/how-to-debug-c-program) your code... – Joe Oct 08 '13 at 17:25
  • once i started to write it, i couldn't delete it :/ – Ka Putsu Oct 08 '13 at 17:26
  • Shouldn't your `malloc()` calls be paired with calls to `free()`? – Cyclonecode Oct 08 '13 at 17:26
  • `sizeof(char)` is `1`. – Carl Norum Oct 08 '13 at 17:27
  • i thought, since the allocation is happening in a function, it would free the allocated space, after the execution of the function – Ka Putsu Oct 08 '13 at 17:28
  • Just for your own sanity, [consider a different approach to `ch()`](http://ideone.com/SqQrlE). – WhozCraig Oct 08 '13 at 17:45
  • guess i need a lot more program practice to be able to use it appropriately :) thanks for the code – Ka Putsu Oct 08 '13 at 17:49
  • 2
    You could replace the contents of your `ch` function with `if (x < 0 || x > 19) return 0;` followed by `return "0123456789ABCDEFGHJ"[x];`. – lurker Oct 08 '13 at 17:51
  • In `C` you are on your own to manage dynamic memory. There is not automatic freeing until your program exits. So for every `malloc` you need `free`. – lurker Oct 08 '13 at 17:53
  • **Always** check the return value of malloc() and friends. Also realloc() is a [tricky beast to use correctly](http://stackoverflow.com/questions/3850749/does-realloc-overwrite-old-contents) - I try to avoid it if possible. – Digital Trauma Oct 08 '13 at 18:23

2 Answers2

0

The best place to start is probably freeing all of the memory you've allocated. Granted, I don't know if that will actually fix anything but...

A problem is that baseB returns a malloc'd pointer which is then passed into functions and never freed. Instant leak.

Try something like this:

// At the top of main
char *tmp = NULL;
// All that code until the first baseB
tmp = baseB(y, base);
printf("%s ", tmp); // Or the corresponding is_pal call
free(tmp);
tmp = NULL;

And so on.

Michael
  • 2,181
  • 14
  • 14
0
num=realloc(num,sizeof(char)* strlen(num)+2);

You modifying local copy of 'num' pointer. Caller function will still have old unmodified address. If address will ever change at least once - you boned.

Since the code is already a mess, easiest version (least modifications) would be:

char *append( int x, char* num){
    num=realloc(num,sizeof(char)* strlen(num)+2);
    num[strlen(num)+1] = '\0';
    num[strlen(num)] = ch(x);
    return num;
}

char * baseB(int x, int base){
    int mult=1,lim=1,i;
    char *num;
    num = malloc(sizeof(char));
    num[0] = '\0';
    while(x/(mult*base)){
        mult*=base;
        lim++;
    }
    for(i=0;i<lim;i++){
        num=append(x/mult,num);
        x %= mult;
        mult/=base;
    }
    return num;
}

Aside from that, as comments suggests - debug for once! Debugger is better than SO (at least in this kind of cases).

keltar
  • 17,711
  • 2
  • 37
  • 42
  • thank you for the suggestion, it worked! i wasn't able to Debug it, and also thought, since it was a call by reference, it would work, which clearly wasn't the case :) thanks for the mess call :)) – Ka Putsu Oct 09 '13 at 09:26
  • Whatever you pass to C function, it's always pass-by-value (in this case it's pointer's value - you can change dereferenced value, but not pointer itself - it's just a copy of it). – keltar Oct 09 '13 at 09:59