0

I am working on building a threads library and for some reason have run into a simple malloc problem that I can't fix right now. I'm sure it's something simple I'm just missing it.

In my main.c I have the following code:

//declare testSem
tasem_t testSem;


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

    //initialize testSem
    ta_sem_init(&testSem, 5);
    //wait test
    ta_sem_wait(&testSem);

the relevant code in my thread library is as follows:

void ta_sem_init(tasem_t *sema, int value)
{

    //malloc the semaphore struct
    sema = malloc(sizeof(tasem_t));

    //error check
    if(sema == NULL)
    {
        printf("could not malloc semaphore");
        exit(0);
    }

    //initialize with the given value
    sema->val = value;
    printf("SemaVal = %i\n", sema->val);
}

void ta_sem_wait(tasem_t *sema)
{
    printf("SemaVal = %i\n", sema->val);

    if(sema->val <= 0)
    {
        //not done yet
        printf("SWAPPING\n");
    }
    else
    {
        printf("SemaVal = %i\n", sema->val);
        sema->val = sema->val + 1;
    }
}

Here is the struct from my header file:

//struct to store each semas info
typedef struct tasem_t_struct
{
    //value
    int val;
        //Q* Queue
        //int numThreads


}tasem_t;

The output I get from this is:

SemaVal = 5 SemaVal = 0 SWAPPING

So evidently, I'm not mallocing my struct correctly as the value inside is lost once I go out of scope. I know I must just be forgetting something simple. Any ideas?

tknickman
  • 4,285
  • 3
  • 34
  • 47
  • "I'm sure it's something simple I'm just missing it." -- Maybe, or maybe it's some deep, fundamental misunderstanding. After all, from an experienced (or even not so experienced) C programmer's perspective, it's obvious that `ta_sem_init` leaks memory and makes no sense. – Jim Balter Apr 07 '13 at 19:50
  • @JimBalter It was just a simple overlooked error. I understand it and yes it absolutely did leak memory. – tknickman Apr 07 '13 at 19:54
  • Finally, to reiterate my first point: assigning the value of `malloc` to a **parameter** is not a matter of overlooking something, it's a fundamental misunderstanding of C and how to program in it. – Jim Balter Apr 07 '13 at 20:08
  • Sometimes you write so much code you miss some small things. I'm sure you too have pulled out your hair over simple mistakes. Happens to the best of us. – tknickman Apr 07 '13 at 20:39
  • I have never made the sort of mistake you made here, nor would I hire or work with someone who did; it is not at all a small thing. If I did make such a mistake, I would immediately see it upon looking at the code. I certainly wouldn't run to SO before doing even the most rudimentary debugging. – Jim Balter Apr 07 '13 at 20:44
  • 1
    We all have to learn sometime. Thanks for the help and I'm sure I won't be making the mistake again. – tknickman Apr 07 '13 at 21:02

1 Answers1

3

You can't seem to decide who's responsible for allocating your tasem_t structure. You have a global variable for it and pass its address to ta_sem_init. But then you have ta_sem_init dynamically allocate a brand new tasem_t structure, saving its address to sema, a local function argument, so that address gets lost when it falls out of scope.

Pick one, either:

Community
  • 1
  • 1
jamesdlin
  • 81,374
  • 13
  • 159
  • 204
  • Yep, that was it. To many variables in to many places. Thanks for the help! – tknickman Apr 07 '13 at 19:52
  • These sorts of errors cannot arise if people follow the simple practice of writing a short descriptive comment for each function before coding the function ... either `// allocates an initialized tasem_t and returns a pointer to it` or `// takes a pointer to an uninitialized tasem_t and initializes it` – Jim Balter Apr 07 '13 at 20:04