Getting several errors including "illegal use of this type as an expression"

Question

I'm getting several errors when trying to run the *HNum_clone function. Probably missing something really stupid? HNum is supposed to represent a number of any length.

typedef struct _HNum 
{
    char *a;
}HNum;

/*
 * Allocates a new HNum and sets its value to 0.
 * RETURN VALUE:
 *   Returns a pointer to the new number, or NULL if the allocation failed.
 */
HNum *HNum_alloc()
{
    HNum *newNum;
    newNum->a = (char*)malloc(2*sizeof(char));
    if(!newNum->a) return NULL;
    newNum->a = "0";
    return newNum;
}

/*
 *Allocates a new HNum with the same value as hnum. It is the caller's
 * responsibility to free the returned HNum.
 * RETURN VALUE:
 *   Returns a pointer to the new number, or NULL if the allocation failed.
 */
HNum *HNum_clone(const HNum *hnum)
{
    if(!hnum)
    {
        return NULL;
    }
    HNum *newNum = HNum_alloc();
    if (hnum->a) {
        strcpy(newNum->a, hnum->a);
    }
    else
    {
        newNum->a = NULL;
    }
    return newNum;
}

*Changed to the code below, still getting the same errors:

HNum *HNum_alloc(void)
{
    HNum *newNum = (HNum*)malloc(sizeof *newNum);
    if(newNum != NULL)
    {
        newNum->a = (char*)malloc(2);
        if(newNum->a != NULL)
        {
            strcpy(newNum->a, "0");
            return newNum;
        }
    free(newNum);
    }
  return NULL;
}

HNum *HNum_clone(const HNum *hnum)
{
    if(!hnum)
    {
        return NULL;
    }
    HNum *newNum = (HNum*)malloc(sizeof *newNum);
    if(newNum != NULL)
    {
        newNum->a = (char*)malloc(strlen(hnum->a)+1);
        if(newNum->a != NULL)
        {
            strcpy(newNum->a, hnum->a);
            return newNum;
        }
    }
    return NULL;
}

Changed HNum_clone to the following, seems to be working now. Looks like the problem was that HNum *newNum wasn't defined at the start of the function...

HNum *HNum_clone(const HNum *hnum)
{
    HNum *newNum = HNum_alloc();
    if(!hnum)
    {
        return NULL;
    }
    if(newNum != NULL)
    {
        free(newNum->a);
        newNum->a = (char*)malloc(strlen(hnum->a)+1);
        if(newNum->a != NULL)
        {
            strcpy(newNum->a, hnum->a);
            return newNum;
        }
    }
    return NULL;
}

In the line "HNum *newNum = HNum_alloc();" - "'HNum': illegal use of this type as an exception" and "'newNum': undeclared identifier". "strcpy(newNum->a, hnum->a);" - "'newNum': undeclared identifier" and "'strcpy': too few arguments for call". — AGPrometheus, Jun 29 '14 at 09:26
`newNum->a = (char*)malloc(sizeof(hnum->a)+1);` --> `newNum->a = (char*)malloc(strlen(hnum->a)+1);` — BLUEPIXY, Jun 29 '14 at 10:30
try `if(!hnum){ return NULL; }` replace to `if(!hnum){ return NULL; } else {//rest part}` — BLUEPIXY, Jun 29 '14 at 10:46
Must occur at the beginning of the block of variable declarations in MSVC(C89). — BLUEPIXY, Jun 29 '14 at 10:53

score 5 · Answer 1 · answered Jun 29 '14 at 09:23

5

This code:

HNum *HNum_alloc()
{
    HNum *newNum;
    newNum->a = (char*)malloc(2*sizeof(char));
    if(!newNum->a) return NULL;
    newNum->a = "0";
    return newNum;
}

has almost more wrong lines than correct ones, I'm afraid. :/ You don't specify which lines you get the compiler warning on, but that almost doesn't matter: it will never work with that code in there.

The function should be (void), empty parentheses don't mean "no arguments" i C.
You must allocate memory for newNum.
You should never use sizeof (char), it's always 1.
You overwrite the allocated pointer with the pointer to a string constant "0".

So, it should be something like:

HNum * HNum_alloc(void)
{
  HNum *newNum = malloc(sizeof *newNum);
  if(newNum != NULL)
  {
    newNum->a = malloc(2);
    if(newNum->a != NULL)
    {
      strcpy(newNum->a, "0");
      return newNum;
    }
    free(newNum);
  }
  return NULL;
}

Instead of the inner allocation, you could use strdup(). Note, though, that it's much easier to write an efficient number implementation if you store the amount of allocated space in the string buffer, then you can avoid re-allocating it. Also you should allocate more than the required number of digits, again to prevent too frequent re-allocations (which are costly).

Similar errors occur in HNum_clone(), you must allocate memory for both the top-level HNum structure, and for the data pointed at by a. Neither happens magically.

answered Jun 29 '14 at 09:23

unwind

391,730
64
469
606

Sorry I'm really bad at this :\ I tried to change HNum_clone() but it's still giving the same errors. I'll edit the changed code into the question since it won't let me write it here. – AGPrometheus Jun 29 '14 at 10:03
@AGPrometheus Are you compiling this as C++? I see you added a bunch of pointless casts. Also, your use of `sizeof` in the `clone()` function is wrong, it doesn't work like that, you need `strlen()`. – unwind Jun 29 '14 at 10:09
I'm compiling in Visual Studio. I just started studying C, the teacher said to use this program and just add .c in the file names... Changed to strlen(). It's still giving the same errors on the line "HNum *newNum = (HNum*)malloc(sizeof *newNum);" in HNum_clone(). – AGPrometheus Jun 29 '14 at 10:14
2

@AGPrometheus It must be compiling as C++ if you get warnings/errors without the casts. In C, the casts are [a bad idea](http://stackoverflow.com/a/605858/28169), but in C++ they are required. I think it's an even worse idea to compile as C++ when you're trying to learn C, it will adds lots of confusion. – unwind Jun 29 '14 at 10:23
I agree, but that's what he wants me to do, and he taught us to use malloc with casts. But is that the only problem with the code now? – AGPrometheus Jun 29 '14 at 10:28
“empty parentheses don't mean ‘no arguments’ in C.” Yes, that _is_ what they mean for function definitions, only declarations without a definition with empty parentheses mean “indeterminate number of (default-promoted) arguments”. I wouldn't object to suggest it as somewhat cleaner or good coding style, but it's definitely not an error. – mafso Jun 29 '14 at 10:35
@mafso Fine; I still don't think it's a good idea to use different notations for the declaration (prototype) if present, and the definition. – unwind Jun 29 '14 at 11:09
That's what I meant with “somewhat cleaner or good coding style”… – mafso Jun 29 '14 at 11:28

score 0 · Answer 2 · answered Jun 29 '14 at 11:41

try this :

HNum *HNum_clone(const HNum *hnum)
{
    if(!hnum)
    {
        return NULL;
    } else {
        HNum *newNum = (HNum*)malloc(sizeof *newNum);
        if(newNum != NULL)
        {
            newNum->a = (char*)malloc(strlen(hnum->a)+1);//hnum->a != NULL
            if(newNum->a != NULL)
            {
                strcpy(newNum->a, hnum->a);
                return newNum;
            }
        }
    }
    return NULL;
}

Getting several errors including "illegal use of this type as an expression"

2 Answers2