Finding first recurring letter in each string of a given set First input is the number of strings ex: 5
Second input is a set of strings ex: hello world how are you  

Output the letter in the string ex: 'l' in string 1  

Below is my code:  

#include &lt;stdio.h&gt;
#include &lt;stdlib.h&gt;

void find_first_recurring_letter(char **str,int r,int c);

int main()
{
    int r,i;
    printf("Enter the number of strings to be entered");
    scanf("%d",&amp;r);
    /*dynamic allocation for str*/
    char **str=(char**)malloc(r*sizeof(char*));
    for(i=0;i&lt;r;i++)
        str[i]=(char*)malloc(50*sizeof(char));
    for(i=0;i&lt;r;i++)
    {
        printf("Enter the string %d",i+1);
        gets(str[i]);
    }
    for(i=0;i&lt;r;i++)
        printf("%s\n",str[i]);
    find_first_recurring_letter(str,r,50);
    for(i=0;i&lt;r;i++)
       free(str[i]);
    return 0;
}


void find_first_recurring_letter(char **str,int r,int c)
{
    char *ptr,*sptr,ch[r];
    int indx[r];
    int i;
    for(i=0;i&lt;r;i++)
    {
        indx[i]=0;
        ch[i]='\0';
        for(ptr=&amp;str[i][0];*ptr!='\0';ptr++)
        {
            for(sptr=&amp;ptr[1];*sptr!='\0';sptr++)
            {
                if(*ptr==*sptr)
                    ch[i]= *ptr;
                    indx[i]=1;
            }
        }
    }
    for(i=0;i&lt;r;i++)
    {
        if(ch[i]!='\0')
            printf("'%c' in string %d\n",ch[i],indx[i]+1);
    }

}
Better names

The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.

Unused variables

We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.

Strictly speaking, we also never use indx[i]. If ch[i] is '\0',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace

        printf("'%c' in string %d\n",ch[i],indx[i]+1);


by

        printf("'%c' in string %d\n", ch[i], 2);


without changing the programs logic.

Simpler pointer logic

Instead of ptr = &amp;str[i][0], which is ptr = &amp;(*(str[i])) use ptr = str[i]. Instead of sptr = &amp;ptr[1], just use sptr = ptr.

Again, we could call them current and next or similar.

By the way, all indx[i] are 1  (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.

Get rid of additional memory

If we printf the recurring character, we can get rid of ch and indx:

            if(*ptr==*sptr)
                printf("....");
                indx[i]=1;


Always use braces

Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.

You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.

If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.

Prefer late C99 declarations if possible

That way we can keep the scope of our variables short, e.g.

for(int i = 0; i &lt; number_of_strings; ++i) {
    for(char *current = str[i]; *current != '\0'; current++) {
        for(char *next = current + 1; *next != '\0'; next++) {
            if(*current == *next) {
                printf("'%c' in string %d\n", *current, i);
            }
        }
    }
}


It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.

Use const type* for inputs that are not supposed to change

That way we cannot accidentally modify your input values, e.g.

str[i][k] = '\0'


would not compile if str was const char**.

Check malloc's return

We should really check whether malloc returns NULL.

Split functions

find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:

const char * find_first_recurring_letter(const char * haystack) {
    for(const char * current = haystack; *current != '\0'; current++) {
        for(const char * next = current + 1; *next != '\0'; next++) {
            if(*current == *next) {
                return current;
            }
        }
    }
    return NULL;
}

void find_first_recurring_letters(const char ** strings, int number_of_strings) {
    for(int i = 0; i &lt; number_of_strings; i++) {
        const char * result = find_first_recurring_letter(strings[i]);

        if(result) {
            printf("'%c' in string %d\n", *result, i);
        }
    }
}


We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.