I have a program to compile,need help.

/*This program is to permute some letters of alphabet.
However i can not compile it .
There are always many errors.
your help will be appreciated deeply.
I am a new guy in Ubuntu .*/

Code:

---

#include<stdio.h>
#include<string.h>
int L;
void insort(int n,char s[],char c[]);


void insort(int n,char s[],char c[])
{
char d[50];
char str1[50],*str2;
int i;
if(n==0) s[n]=c[n];
else{ for(i=n;i>=0;i--)
{ memcpy(d,s,strlen(s));
d[strlen(s)]='\0';
memcpy(str1,d,i);
str1[i]='\0';
str2=d+i;
strcat(str1,c[n]);
strcat(str1,str2);
//d=str1;
memcpy(d,str1,strlen(str1));
d[strlen(str1)]='\0';
if (n==L)printf("%s\n",d);
}
}
if (n+1<=L)
insort(n+1,s,c);


}




int main(){ char str[1000];
str[0]='\0';

char S[]="abcd";

L =strlen(S);

insort(0,str,S);

return 0;

}

---

If you got errors, you should be able to say what they are.

And I doubt anyone is going to look very carefully at the code until it's indented in some reasonable way.

errors are follow:
In function "insort"
warning: passing argument 2 of ‘strcat’ makes pointer from integer without a cast.

Code:

---

strcat(str1,c[n]);

---

You're passing a char here instead of a char*

so ,how can I correct this mistake?

Are you trying to append a single character there, or do you want the whole string beginning at position n?

I am trying to append a single character there

Code:

---

int L;

int main() {
...
  L = strlen(S);
...
}

---

You're using a global variable to pass in strlen(S)?? Something that could have easily been done inside the function? Why? This is terrible style.

Code:

---

if (n == 0)
  s[n]=c[n];

---

If you know that 'n' is 0 here, you may as well just write s[0] = c[0]; Its clearer to me at least. On a quick glance a reader might get confused.

Code:

---

memcpy(d,s,strlen(s));
d[strlen(s)]='\0';
...
memcpy(d,str1,strlen(str1));
d[strlen(str1)]='\0';

---

You do realize those behave exactly the same as

Code:

---

strcpy(d, s);
strcpy(d, str1);

---

right? Except the strcat's are going to be easier to read and more efficient (no need to loop through the string to calculate it's length).

Code:

---

memcpy(str1,d,i);
str1[i]='\0';

---

Once again, this is equivalent to

Code:

---

strncpy(str1, d, i);

---

It's easier to read, use it!

Code:

---

strcat(str1,c[n]);

---

Riiight, pass a character to a function that expects a null-terminated string. THAT's a good idea.

Code:

---

strncat(str1, &c[n], 1);

---

Oh wow, much better.

Code:

---

str2=d+i;
...
strcat(str1,str2);

---

Really? The WHOLE reason you put an amazingly descriptive 'str2' in there is so you can temporarily store 'd + i' for no reason? If you're going to use variable names like 'str2' you may as well just use 'd + i' in the call to strcat(str1, d + i);

You don't think it's just coincidence that, since you're working with strings, the string handling functions are useful, do you?

Also, the code as you posted it won't even work for at least one reason that I can see. On the first iteration of the loop you are overwriting the null-terminator in 's':

Code:

---

if(n==0) s[n]=c[n];

---

On the very next iteration of the loop, you assume you can use strlen(s), which obviously isn't true because you just blew away the null-terminator!

Code:

---

memcpy(d,s,strlen(s));

---

And finally, FOR THE LOVE OF GOD LEARN TO INDENT PROPERLY. The forums have a freakin' preview button, why the hell can't anyone figure out how to use it!?

Code:

---

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

void insort(int n, char s[], char c[])
{
  char d[50], str1[50];
  int i;
 
  if (n == 0)
      s[0]=c[0];
  else {
      for (i = n; i >= 0; i--) {
        strcpy(d, s);
        strncpy(str1, d, 1);
        strncat(str1, &c[n], 1);
        strcat(str1, d + i);
        strcpy(d, str1);
       
        if (c[n] == '\0')
            printf("&#37;s\n", d);
      }
  }
 
  if (c[n] != '\0')
      insort(n+1, s, c);
}

int main(void) {
  char str[1000] = "";
  char S[] = "abcd";
 
  insort(0, str, S);
  return 0;
}

---

Was that so hard? Now go fix it.
(Btw, I still have no idea what this function is supposed to do, your choice of variable names and documentation is exquisite)

Quote:

---

Originally Posted by wxjeacen

I am trying to append a single character there

---

In that case, you shouldn't use strcat() since that appends a whole string instead of a single character.
I haven't finished my coffee yet, but I believe this should work:

Code:

---

size_t len = strlen( str1 );
str1[len] = c[n];
str1[len + 1] = '\0';

---

Thanks very much ,masters.


I have learned a lot from your replies.


however ,there is still something wrong with my algorithm,


maybe ,the algorithm of permuting strings is not perfect.


I think ,i have to find another way to complete the algorithm.

Quote:

---

Originally Posted by wxjeacen

however ,there is still something wrong with my algorithm,


maybe ,the algorithm of permuting strings is not perfect.


I think ,i have to find another way to complete the algorithm.

---

What's wrong with it?
It might be that you're overflowing the buffer, since you're using strcat() which doesn't know the size of your string buffer.

however, i have had defined the array str1[50] and d[50],how can they be overflowed?

Quote:

---

Originally Posted by wxjeacen

however, i have had defined the array str1[50] and d[50],how can they be overflowed?

---

By putting 50 or more letters in them. It's hard to say whether or not that happens, just by looking.