-1

My coding assignments came with it's header file, meaning we need to use the same data types, and not vary anything. There is a lot of pointers, (mainly a lot of void *). Meaning things are confusing, more than difficult. we have to do a separate function, just to increment the value referenced by a pointer. But given the nature of program, I don't want to constantly make new pointers.

The code is as follows:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
void* intal_create(const char* );
void* intal_increment(void* );

void *intal_create(const char* str)
{
    int a;
    a=atoi(str);
    return &a;
}

void *intal_increment(void *intal)
{
    int *a= (int *)intal;//new pointer;
    ++*a;
    //value referenced has been incremented;
    *(int *)intal=*a;
    return intal;
}

int main()
{
    int * x;// void * return a pointer, need a pointert to int to pick it up
    char *dummy;
    gets(dummy);
    x=(int *)intal_create(dummy);
    printf("integer return is %d\n",*(int *)x);
    printf("address stored is %p\n",(int *)x);
    x=(int *)intal_increment(x);
    printf("integer return is %d\n",*(int *)x);
    printf("address stored is %p\n",(int *)x);
}

I wanted x to be the parameter called, and also for it to store the return value. The printf address is merely for my understanding.

The segmentation faults never end, and from my understanding, I'm just returning a pointer and asking a pointer to stop the return pointer

eyllanesc
  • 235,170
  • 19
  • 170
  • 241
Mr. Johnny Doe
  • 99
  • 11
  • 3
    `return &a;` is a bug. Local variables of functions stop existing when the function returns, and you cannot use any pointers to them – M.M Jul 07 '18 at 09:22
  • 2
    `gets(dummy);` is another bug (in multiple ways), this is probably the cause of your segmentation fault – M.M Jul 07 '18 at 09:22
  • 1
    If your compiler isn't warning you about stuff like this, you really need to up the warning options. `-Wall -Wextra` is good for gcc and clang. – Shawn Jul 07 '18 at 09:36
  • Here `gets(dummy);` input is read to were `dummy` points, which is "nowhere". This invokes the infamous Undefined Behaviour. From then on anything can happen, from seemingly working to crash. – alk Jul 07 '18 at 09:38
  • The first problem is that you should not return a pointer when memory it points to is in stack, suggest you to return a instead of return &a from the intal_create function. In addition to that for dummy you need to allocate memory. You are just creating pointer, memory is not allocated. – yadhu Jul 07 '18 at 09:39
  • So how would you suggest I initialise "Dummy". and if I can't return the address of local variable a, is it better to return value and have a pointer's pointed value change? I apologise for the question being low-level, along with buggy code, I'm an electronics student and have never been the best at coding, All this was from an extra credit course that I took up – Mr. Johnny Doe Jul 07 '18 at 09:40
  • Assuming your maximum string input is of length 20, You can initialize as "char dummy[21] = {0};" – yadhu Jul 07 '18 at 09:44
  • "*we have to do a separate function, just to increment the value referenced by a pointer.*" so why the call to `gets()` at all? – alk Jul 07 '18 at 09:45
  • because another part specified by header file was "input string and convert it integer". void* intal_create(const char* str). It seems like, thanks to a character pointer, I can't actually do that. Since I can't create an array. – Mr. Johnny Doe Jul 07 '18 at 09:50
  • If you want return a pointer from the intal_create then you need to allocate memory in the heap by using malloc() function. – yadhu Jul 07 '18 at 09:53
  • ... or define `a` as `static`, givning away thread safeness with this, which probably is not an issue in the OP's case. – alk Jul 07 '18 at 09:53
  • Why can't you create an array?, without allocated memory where gets will store the input string? string is nothing but a char array with null character at the end. – yadhu Jul 07 '18 at 10:00
  • the void* intal_create(const char* str) is really confusing me, am I supposed to create an array and initialise a pointer to it? – Mr. Johnny Doe Jul 07 '18 at 10:04
  • No, you can do this "int *a = (int *)malloc(sizeof(int));" and then assign the return value of atoi function to *a i.e "*a = atoi(str)" then return a. – yadhu Jul 07 '18 at 10:07
  • got it Yadhunandana. Thanks a lot. Can I ask why that made such a difference? – Mr. Johnny Doe Jul 07 '18 at 10:11
  • You can accept my answer then. :-), variables allocated in stack(I mean inside the function) valid only inside the function. if you returning the pointer then you need to allocate in heap. As far as gets function is concerned, It will write the input value to location pointed by passed pointer. – yadhu Jul 07 '18 at 10:16
  • [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/q/1694036/995714) @Yadhunandana you [shouldn't cast the result of `malloc`](https://stackoverflow.com/q/605845/995714), and [shouldn't use `atoi`](https://stackoverflow.com/q/17710018/995714) – phuclv Jul 07 '18 at 10:46

1 Answers1

0

By incorporating all the comments. Mainly allocating memory to dummy before passing it gets() function and allocating memory in heap for the return pointer of intal_create.These two fixes solve the issue. Have a look at the following code for reference.

    #include<stdio.h>
    #include<stdlib.h>
    #include<string.h>
    void* intal_create(const char* );
    void* intal_increment(void* );

    void *intal_create(const char* str)
    {
        int *a = (int *)malloc(sizeof(int));
        *a = atoi(str);
        return a;
    }

    void *intal_increment(void *intal)
    {
        //Here i am not allocating
        int *a = (int *)intal;//new pointer;
        (*a)++;
        return intal;
    }

int main()
{
    int * x;// void * return a pointer, need a pointert to int to pick it up
    char dummy[20] = {0};
    fgets(dummy,5,stdin);
    x = (int *)intal_create(dummy);
    printf("integer return is %d\n",*x);
    printf("address stored is %p\n",(void*)x);
    x=(int *)intal_increment(x);
    printf("integer return is %d\n",*x);
    printf("address stored is %p\n",(void *)x);
    //Make sure you deallocate the memory allocated in the intal_create function.
    free(x);
}
yadhu
  • 1,253
  • 14
  • 25
  • 2
    `gets()` is not C (any more). Use `fgets()` instead. – alk Jul 07 '18 at 10:20
  • 2
    Also all these casts are useless. Just drop them. – alk Jul 07 '18 at 10:20
  • 1
    And add casting where it *is* needed: The `p` conversion specifier is defined for `void*` **only**. – alk Jul 07 '18 at 10:22
  • I didn't want to do lot changes to given sample, So i have modified only buggy parts . – yadhu Jul 07 '18 at 10:23
  • 2
    Strictly speaking this `... %p\n", x);` and this `... %p\n",(int *)x);` invokes undefined behaviour. – alk Jul 07 '18 at 10:24
  • why ... %p\n", x); this invokes a undefined behavior? – yadhu Jul 07 '18 at 10:29
  • "*why ... %p\n", x); this invokes a undefined behavior?*" See here: https://stackoverflow.com/questions/51221690/returning-pointers-back-to-itself#comment89424887_51222152 – alk Jul 07 '18 at 10:32
  • The C Standard says of `%p` that: ["The argument shall be a pointer to void,"](http://port70.net/~nsz/c/c11/n1570.html#7.21.6.1p8) and that ["if any argument is not the correct type for the corresponding conversion specification, the behavior is undefined."](http://port70.net/~nsz/c/c11/n1570.html#7.21.6.1p9) You need `printf("address stored is %p\n",(void *) x);` – ad absurdum Jul 07 '18 at 10:33
  • Thanks. I didn't know.. I will update with the typecast type. – yadhu Jul 07 '18 at 10:36
  • void * really is confusing. Thanks for the void* type cast advice. I've been personally struggling with maximising the efficiency/ benefits of using void *. I just realised that we also need addition of numbers larger than a hundred digits. And casting everything to int is an issue. Still trying to figure that out – Mr. Johnny Doe Jul 07 '18 at 12:06