working with strings in c- expanding, replacing, storing and printing out strings

Question

I have a simple C program that is supposed to expand the string FX according to the following rules:

X -> X+YF+
Y-> -FX-Y

The program is as follows:

#include <stdio.h>
void expandOnce( )
{
     char* s = (char*)malloc(sizeof(char)*100);
     int i=0;
     char str[]="FX";
     while(str[i]!='\0')
     {
           if(str[i]=='X')
           {
               s[i]="X+YF+";
           }
             if(str[i]=='Y')
           {
               s[i]="-FX-Y";
           }
           i++;
     }
          printf("%s",s);
}


void expandNtimes (int n)
{

for (int i =0; i < n; i++){
expandOnce();
}

}
int main(){
   expandNtimes(2);
    return 0;
}

If the program executes correctly, it should print out:

  FX+YF++-FX-YF+

This is after 2 expansions of FX using the above rules. Questions: 1. How can you make this work without printing out gibberish? 2. How can I return the expanded array from expandNtimes() and expandOnce() so that I can, for instance, use it in the main function?

I try this but I keep getting memory errors.

After your suggestions, I made the following changes:

#include <stdio.h>
#include <string.h>
#define NTIMES  2
#define MAXC  100

void expandOnce( )
{
    char s[MAXC];
     int i=0;
     s[0]='F';
     s[1]='X';

     while(s[i]!='\0')
     {
    if(s[i]=='X'){
    //shift all letters to the right by 5 steps
        for (i; i < MAXC; i++){ //warning: statement with no effect (Wunused value)
            s[i + 5]= s[i];
            if(s[i]=='\0'){
            break;}
                  }
    //Fill in the expansion
        s[i]='X';
        s[i+1] ='+';
        s[i+2] ='Y';
        s[i+3]='F';
        s[i+4] ='+';
    }
  if(s[i]=='Y'){
  //shift all letters to the right by 5 steps
        for (i; i < MAXC; i++){ //warning: statement with no effect (Wunused value)
            s[i + 5]=s[i];
            if(s[i]=='\0'){
            break;}
                }
    //Fill in the expansion
        s[i]='-';
        s[i+1] ='F';
        s[i+2] ='X';
        s[i+3]='-';
        s[i+4] ='Y';
    }
             i++;
     }
          printf("%s\n",s);
           printf("Me\n");
}


void expandNtimes (int n)
{

for (int i =0; i < n; i++){
expandOnce();
}

}
int main(){
expandOnce();
expandNtimes(NTIMES);
return 0;
}

Both of my for loops have a warning warning: statement with no effect (Wunused value) which I do not understand. Finally, I get this error:

Segmentation fault (core dumped)

Is it allowed to do a for-loop inside a while-loop?

Answer 1

Oh Boy... Do you have a whole list of "you can't do that..." in your code.

To begin with there is no need to cast the return of malloc, it is unnecessary. See: Do I cast the result of malloc?.

char *s = malloc (sizeof *s * 100);

is all that is needed (note: if you always use sizeof dereference_pointer you will always have your type size correct.

This brings up another point, if you need constants #define one (or more) or use a global enum to accomplish the same thing, e.g.

#define NTIMES  2
#define MAXC  100

or

enum { NTIMES = 2, MAXC = 100 };

Then you can use them like,

char *s = malloc (sizeof *s * MAXC);

and

expandNtimes(NTIMES);

By defining constants, you then have one convenient place at the top of your code to make changes if needed in the future and not have to pick through each allocation or function call changing all your hardcoded magic numbers.

Your Memory Leak

You allocate for s in expandOnce, but never free s and don't return a pointer so that it could be later freed in main() (or any other calling function).

You should include free (s); after printf("%s",s); (which you should also include a newline in to prevent multiple outputs from running together, and so that your program is POSIX compliant -- outputting a final '\n'), e.g. printf("%s\n",s);

Moreover, "Why are you dynamically allocating anyway?" Just use a fixed buffer, you are not trying to return it. Just use:

char s[MAXC] = "";

s is Uninitialized and You Leave s[0] Uninitialized

s[i]="X+YF+"; and s[i]="-FX-Y"; write 4-characters to s from index 1. s[0] is never initialized which causes any subsequent attempt to read from s invoke Undefined Behavior. You cannot simply assign the pointer to string literal "X+YF+"; to a character s[i] -- your compiler should be screaming warnings and errors at you.

You can copy the literal to s with:

strcpy (s, "X+YF+");

or you can simply loop repeatedly copying as you go, e.g.

int i;
for (i = 0; str[i]; i++) /* loop over each char in str */
    s[i] = str[i];       /* assign to s */
s[i] = 0;                /* don't forget to nul-terminate s */

Compile with Warnings Enabled

Always compile with warnings enabled, and do not accept code until it compiles cleanly without warning. To enable warnings add -Wall -Wextra to your gcc or clang compile string. (add -pedantic for several additional warnings). For clang, instead you can use -Weverything. For VS (cl.exe on windoze), add /W3 (or use /Wall to enable all warnings).

Read and understand each warning. They will identify any problems, and the exact line on which they occur. You can learn as much about coding by simply listening to what your compiler is telling you as you can from most tutorials.

Put these suggestions to use and then edit your question (or ask a new one) if you have further problems.

---

Additional Comments After Edit

Gakuo, it is obvious you are still struggling to sort out string/character array handling in order to make the expansions work. As I mentioned in the comments, whenever you have a problem like this, do not pick up the keyboard and start pecking away hoping that you can tinker long enough that divine intervention will somehow guide you to a solution (it won't...).

Instead first pick up a 8 1/2 x 11 sheet of paper (graph paper works really well) and write out your original string, and then plan each step required to get you from your original string to the final string you need (don't forget to represent the '\0' in your layout as well)

For example, you could start with your original string:

+---+---+---+
| F | X |\0 |
+---+---+---+

The next step would be to represent the shift to make room for your expansion (you leave the original character in place since you will simply overwrite it):

+---+---+---+---+---+---+---+
| F | X |   |   |   |   |\0 |
+---+---+---+---+---+---+---+

Now it is a matter of copying "X+YF+" to the position of 'X' resulting in:

+---+---+---+---+---+---+---+
| F | X | + | Y | F | + |\0 |
+---+---+---+---+---+---+---+

Then simply repeat this process until you have fully exanded N times. As you do, you will be able to determine the pattern for expanding N times -- and that is when it is time to pick up the keyboard.

For your case, rather than manually looping everything, make use of memmove and memcpy to help shift and copy within the string. (you are free to manually shift and copy by looping -- as long as you can find more letters for you loop variable than just 'i'). Letting the memory functions help, you could do something like:

(note: edited to accommodate changing length of s in expandonce())

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

#define NTIMES  2
#define MAXC  100

#define SDEF "FX"       /* string (default) */
#define XEXP "X+YF+"    /* X expansion */
#define YEXP "-FX-Y"    /* Y expansion */

/* returns pointer to 1-past last expansion on success, NULL otherwise */
char *expandonce (char *s)
{
    char *p = s,
        *expanded = NULL;
    size_t lxexp = strlen (XEXP),
        lyexp = strlen (YEXP);

    while (*p) {
        if (*p == 'X') {        /* handle 'X' found */
            size_t lens = strlen (s);               /* get new length of s */
            if (MAXC > lens + lxexp) {                  /* room to expand? */
                size_t lenp = strlen (p);               /* total to shift */
                /* shift chars to right of 'X' by lxexp-1 including '\0' */
                memmove (p + lxexp, p + 1, lenp + lxexp - 1);
                memcpy (p, XEXP, lxexp);                /* copy expansion */
                p += lxexp;                             /* add offset */
                expanded = p;                           /* set return */
            }
        }
        else if (*p == 'Y') {   /* handle 'Y' found */
            size_t lens = strlen (s);                   /* same for 'Y' */
            if (MAXC > lens + lyexp) {
                size_t lenp = strlen (p);
                memmove (p + lyexp, p + 1, lenp + lyexp - 1);
                memcpy (p, YEXP, lyexp);
                p += lyexp;
                expanded = p;
            }
        }
        else    /* handle all other chars */
            p++;
    }

    return expanded;
}

/* returns number of successful expansion loops */
int expandntimes (char *s, int n)
{
    int i = 0;
    char *p = s;

    for (i = 0; i < n; i++)
        if ((p = expandonce (s)) == NULL)
            break;

    return i;
}

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

    char str[MAXC] = "";
    int n = 0;

    if (argc > 1 && strlen (argv[1]) < MAXC - 1)
        strcpy (str, argv[1]);
    else
        strcpy (str, SDEF);

    printf ("string: %s\nX_exp : %s\nY_exp : %s\n\n", str, XEXP, YEXP);

    n = expandntimes (str, NTIMES);

    if (n == 0) {
        fputs ("error: no expansions possible.\n", stderr);
        return 1;
    }

    printf ("%d expansions => %s\n", n, str);

    return 0;
}

(note: this is a minimum example -- you should verify that all corner cases are covered as you step through your design)

Example Use/Output

Your example:

$ ./bin/expand2 "FX"
string: FX
X_exp : X+YF+
Y_exp : -FX-Y

2 expansions => FX+YF++-FX-YF+

A slightly different string:

$ ./bin/expand2 "XY"
string: XY
X_exp : X+YF+
Y_exp : -FX-Y

2 expansions => X+YF++-FX-YF+-FX+YF+--FX-Y

Answer 2

You getting memory errors because you try to write in memory where are you not supposed to do anything.if you want to append string you must use strcat() function! Line s[i]="X+YF+"; will never compile because you try to write multiple characters in place where is allowed only one character. i think it is better to make s global and append charracters one by one to s with strcat until you make string. You can access to global variables in main.Then print string s.