2

I am trying to create a function fib(n) that takes the two characters a and b as initial arguments then prints every single term of the Fibonacci sequence for a given index n

the sequence is described as:

S0=" "
S1="b"
S2="a"
Sn="S(n-1)S(n-2)"

meaning for example the result for n=6 should be :

"b","a","ab","aba",abaab","abaababa"...

My issue is when I run the code the strings get random symbols added to them which if removed would give the desired result and I can't find the reason for that anywhere.

When I run the code and give n the value 6 This is the returned result

here is the code :

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
void combine(char**s,char**ch1,char**ch2)
{
    int size=strlen(*ch1)+strlen(*ch2);
    free(*s);
    *s=(char*)malloc(size*sizeof(char)+1);
    strcpy(*s,*ch1);
    strcat(*s,*ch2);
}
void replace(char**ch1,char**ch2)
{
    free(*ch1);
    *ch1=(char*)malloc(strlen(*ch2)*sizeof(char)+1);
    strcpy(*ch1,*ch2);
}
void fib(int n)
{
    char*a,*b,*s;
    int i;
    printf("S0 = ' '\n");
    if(n>=1)
    {   
        printf("S1 = 'b'\n");
        if(n>=2)
        {
            printf("S2 = 'a'\n");
            if(n>2)
            {
                s=(char*)malloc(sizeof(char)+1);
                b=(char*)malloc(sizeof(char)+1);b[0]='b';
                a=(char*)malloc(sizeof(char)+1);a[0]='a';
                for(i=3;i<=n;i++)
                {
                    combine(&s,&a,&b);
                    printf("S%d = '%s'\n",i,s);
                    replace(&b,&a);
                    replace(&a,&s);
                }
                
            }
        }
    }
}
int main()
{
    int n;
    char *a,*b,*s;
    printf("Give the index at which to stop : ");
    scanf("%d",&n);
    fib(n);
}
Jason Aller
  • 3,541
  • 28
  • 38
  • 38
aimen__
  • 47
  • 6
  • 4
    None of this code is accounting for the NUL byte required at the end of every string. `int size=strlen(ch1)+strlen(ch2);` should have a +1 in there, plus other places too. – Steve Friedl Nov 22 '19 at 03:29
  • @SteveFriedl i thought malloc allocates more memory then needed but i will try to account for the NULL byte and see //update : this unfortunately was not the issue – aimen__ Nov 22 '19 at 03:35
  • `malloc()` allocates *exactly* as much as you ask it to, so if you want the NUL byte (not a NULL pointer) you have to ask for it. Be sure to do this everywhere! – Steve Friedl Nov 22 '19 at 03:39
  • One reason `malloc()` won't automatically allocate a bit more is that it can't tell if you are allocating a string (where that +1 would be helpful), or a binary structure object (where it would be wasteful). – Steve Friedl Nov 22 '19 at 03:41
  • 1
    BTW, this is not C code. There are function reference parameters which do not exist in C. Are you actually trying to write C++ code? – kaylum Nov 22 '19 at 03:43
  • @kaylum no, i am writing in C, can you tell me exactly what part you are talking about? – aimen__ Nov 22 '19 at 03:48
  • `void replace(char*&ch1,char*&ch2)`. The `&` makes the parameters reference parameters. That is a C++ feature and is not supported in C. This should fail to compile with a C compiler. Also, if you have fixed the issues identified by @SteveFriedl and the error still occurs then please update your code with the changes. – kaylum Nov 22 '19 at 03:50
  • @kaylum the issue pointed by steve did not impact the result, the code is compiling and runing but the result is altered somehow,as far `&` i have used this before in C with programs that work. – aimen__ Nov 22 '19 at 03:56
  • 1
    Please show us your latest code. The issues pointed out by steve MUST be fixed before we can proceed. There is no point trying to debug the code you have shown because those errors are major. We need to see the updated code so that we have your latest version and also to check that the new changes are correct and complete. Regarding `&`, do your own research if you like - that's not C code. What compiler are you using? – kaylum Nov 22 '19 at 03:58
  • @kaylum the issue pointd by steve has been fixed and the code has been updated above, i use DEVC++ as my compiler which is one of the advised compilers in my university – aimen__ Nov 22 '19 at 04:03
  • That is C++, not C – Steve Friedl Nov 22 '19 at 04:05
  • That's a C++ compiler and it's building C++ code not C code. So my comment stands - your code is not valid C. That's fine if you are ok with that but just something to be aware of. – kaylum Nov 22 '19 at 04:05
  • @kaylum what changes would you suggest to make it pure C code ? i tried to find passing dynamic strings in functions and only found that form " `*&` " – aimen__ Nov 22 '19 at 04:08
  • @SteveFriedl that's not technically correct. A given `malloc` implementation is free to allocate more bytes than you ask for. Some might do it for byte-alignment, I think some will allocate more space for internal bookkeeping. So if `malloc` doesn't return NULL, it allocated _at least_ as many bytes as you ask for. But, it is very true that accessing beyond what you ask for is UB, that's certainly nothing to count on: https://stackoverflow.com/questions/35110245/why-does-malloc-allocate-more-memory-spaces-than-i-ask-for – yano Nov 22 '19 at 04:18

1 Answers1

4

Doing this in proper C means dumping the & references of C++ and adding one level of pointer indirection. This only needs to be done for parameters whose pointers need to be modified.

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

void combine(char **ps, const char *ch1, const char *ch2)
{
    int size=strlen(ch1) + strlen(ch2);

    free(*ps);

    *ps = malloc(size + 1);

    strcpy(*ps, ch1);
    strcat(*ps, ch2);
}

void replace(char **pch1, const char *ch2)
{
    free(*pch1);

    *pch1 = malloc(strlen(ch2) + 1);

    strcpy(*pch1, ch2);
}

void fib(int n)
{
    printf("S0 = ' '\n");

    if (n >= 1)
    {
        printf("S1 = 'b'\n");

        if (n >= 2)
        {
            printf("S2 = 'a'\n");

            if (n > 2)
            {
                char *s = (char*)malloc(2);
                char *b = (char*)malloc(2); b[0]='b'; b[1] = 0;
                char *a = (char*)malloc(2); a[0]='a'; a[1] = 0;

                int i;
                for (i = 3; i <= n; i++)
                {
                    combine(&s, a, b);

                    printf("S%d = '%s'\n", i, s);

                    replace(&b, a);
                    replace(&a, s);
                }

            }
        }
    }
}

int main()
{
    int n;
    printf("Give the index at which to stop : ");
    scanf("%d",&n);
    fib(n);
}

Looking at combine(), the only pointer that needs to be modified is the first one, so I've changed char &*s to char **ps, and we dereference it with *ps throughout. The other parameters are just "regular" pointers because the pointers themselves don't need to be modified. I made them const char * for good measure.

Also of note are the memory allocations of a and b:

char *b = (char*)malloc(2); b[0]='b'; b[1] = 0;
char *a = (char*)malloc(2); a[0]='a'; a[1] = 0;

We need two bytes each, and the NUL byte after; without this your strings run off into crazy-town. This is the main thing that broke the program.

I think it's not necessary to multiply all the allocation counts by sizeof(char) but maybe that's your personal style.

EDIT:The reason for the appearance of the bizzard symbols is in Function fib(int n) is the not initialising a[1] and b[1] meaning that the dynamic string did not have an end mark \0 thus resulting in weird symbols being attached to the end of every string passed to a function weather it being combine() or replace() this issue is fixed by initialising a[1]=0;b[1]=0;

EDIT: The filename needs to end in .c, not .cpp to compile as C and not C++

EDIT: When I compile the above code, I get:

$ ./a.out
Give the index at which to stop : 6
S0 = ' '
S1 = 'b'
S2 = 'a'
S3 = 'ab'
S4 = 'aba'
S5 = 'abaab'
S6 = 'abaababa'
aimen__
  • 47
  • 6
Steve Friedl
  • 3,929
  • 1
  • 23
  • 30
  • It's not just about changing different characters in the code; they behave differently. Please consider studying the updated code carefully and see if you can follow what it's doing. – Steve Friedl Nov 22 '19 at 05:20
  • @SteveFriedli have actualy tried that way before (replacing `*&` with `**` and done exactly as you described but i always get errors that resemble these In function 'void combine(char**, const char*, const char*)': [Error] invalid conversion from 'void*' to 'char*' [-fpermissive] In function 'void replace(char**, const char*)': [Error] invalid conversion from 'void*' to 'char**' [-fpermissive] which is why i have resorted to using `*&` have you or @kaylum seen the image i included? maybe you have seen the error shown in the image occur before before ? – aimen__ Nov 22 '19 at 05:23
  • The reason is that you're using a C++ compiler, and it's not happy with the `void *` to `char *` conversions. I believe that DevC++ is an environment; which underlying compiler are you using? What is the name of the file you're compiling? – Steve Friedl Nov 22 '19 at 05:31
  • i dont know about any underlying compilers within DevC++ the way i begin writing is by clicking File/new/Source file (or Ctrl+n) you are not given a choice as far as i know, its only when you chose New Project that you are presented with options that include (C Project) or (C++ Project) – aimen__ Nov 22 '19 at 05:39
  • What is the name of the file the above code is found in? – Steve Friedl Nov 22 '19 at 05:40
  • it is Untitled3.cpp – aimen__ Nov 22 '19 at 05:42
  • Aha. `.cpp` is a C++ file. Rename it to `Untitled3.c` and you should be good to go. – Steve Friedl Nov 22 '19 at 05:43
  • new Errors popped up : In function 'fib': ; [Error] 'for' loop initial declarations are only allowed in C99 or C11 mode ; [Note] use option -std=c99, -std=gnu99, -std=c11 or -std=gnu11 to compile your code – aimen__ Nov 22 '19 at 05:48
  • Your compiler is using an older version of the C standard; poke around in your IDE to find out how to pass `-std=c99` as a compiler flag - I don't know how this is done in DevC++. Probably in Project Properties or something? Alternately, change the `for (int i = 0;...)` to `int i;` on a separate line, then `for (i = 0; ...)` - this is a feature of newer C EDIT: I've updated the code above to do this. – Steve Friedl Nov 22 '19 at 05:51
  • i have renamed the file to .c and replaced the C++'s "`*&`" with a "`**`" and fixed all local variables within and updated the code above , the program compiles and runs and i get the exact same result as before, weird sybmbols are still added after every string treatment – aimen__ Nov 22 '19 at 13:06
  • When I compile the above cut-and-pasted code and run it with N=6, I get the answer you say in the question you're looking for. – Steve Friedl Nov 22 '19 at 13:16
  • can you share a link so i can see, which compliler did you use ? – aimen__ Nov 22 '19 at 13:19
  • Sorry, cut-and-paste bug on my part. In `replace()` the line should be `*pch1 = malloc(...);` - the `*` was missing in the original code. – Steve Friedl Nov 22 '19 at 13:28
  • 1
    hello again i have found the problem that you fixed and did not point out it is to initilize `a[1]=0;` and `b[1]=0;` thank you so much my friend i will posting the answer , you helped understand C a little better – aimen__ Nov 22 '19 at 13:43
  • can you add the both the facts that i had to change my file name to .c and make sure to initialise the `a[1]=0;` and `b[1]=0` so i can accept it as a definitive answer to this problem ? – aimen__ Nov 22 '19 at 13:58
  • I did mention the a[1] stuff, look again. But I did add the `.c` filename thing. – Steve Friedl Nov 22 '19 at 14:00
  • i meant emphasize the importance of that initialisation because it was the primary root to this problem and thanks again for you help – aimen__ Nov 22 '19 at 14:01