finding duplicates from the array of pointers

Question

i wanted to find the duplicates present in an array of pointers.the code is shown below.when i run this application it is gining segmentation fault.But when i extract this function,i am able to run it just fine.Can anyone please tell me what could `

when i detect the duplicates,i just put those strings to a file named output.txt.

i discovered that when strcmp is used,its giving this segmentation fault. but when i extract this function and run it on some test code,it works really fine.

      main()
      {
           char *a[20];
           DIR             *dip;
           int i = 0;
           dip = opendir("src/my_folder");
           char *condition_var; 


           while ((dit = readdir(dip)) != NULL)               
           {
           condition_var = dit->name;      

            a[i] = condition_var
                            i++;
            }
            findduplicates(a,i);
       }

      char *findduplicates(char *arr[3],int count)
      {
      int i = 0;
      int j = 0;
      int val = 0;
      FILE *output = fopen("output.txt","w");
      for(i = 0;i<count;i++)
      { 
               j = i+1;
       for(;j<count;j++)
       {
             if(strcmp(arr[i],arr[j])==0)
             {
           printf("The index of a duplicate elemnt is %d\n",j);
           arr[j] = " ";

             }
        }

      }
int k = 0;
while(k<3)
{

   printf("the aarray is %s\n",arr[k]);
   fputs(arr[k],output);
   fputs("\n",output);
   k++;
}

}`

advanced thanks Maddy

How are you calling the function? Are the strings in arr null-terminated properly? — Blagovest Buyukliev
– Blagovest Buyukliev, Commented Jun 21, 2011 at 13:16
@Blagovest---this arr is defined as char *arr[20]; ths arr is then arr is buffred with some file names read from readdir system call,which reads the file names from the irectory and fill in this buffer — maddy
– maddy, Commented Jun 21, 2011 at 13:17
There are other problems with your code but I can't bother to point them out before you post something readable: indent the code properly. Also, what are you passing as parameters? Do you really pass an array of pointers pointing to strings that all have the length 2+1 (1 for the null terminator)? — Lundin
– Lundin, Commented Jun 21, 2011 at 13:21
@Lundin ---- can you please tell me how to post the whole code? — maddy
– maddy, Commented Jun 21, 2011 at 13:22
is there any other way to find out the duplicates in an array of pointers.Please let me know in case this way looks bad — maddy
– maddy, Commented Jun 21, 2011 at 13:36

karadoc · Accepted Answer · 2011-06-21 14:47:46Z

0

char *a[20]; // an array of char pointers. 20 elements in the array.

char *findduplicates(char *arr[3],int count)
// arr is an array of char pointers, with 3 elements in the array.

findduplicates(a, count); // this will just pass 3 of the 20 pointers
// thus when you try to access a[3], it will probably seg-fault.

try this instead:

char *findduplicates(char **arr,int count)

[edit] Maybe I'm wrong about this. I just wrote a small test program, and it wouldn't crash even if declared the function parameter as char *a[0]. But try my suggestion anyway. Memory bugs don't always make stuff crash...

edited Jun 21, 2011 at 14:47

answered Jun 21, 2011 at 14:25

karadoc

2,73123 silver badges21 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

maddy Over a year ago

@kardoc..Is there any othr way by which i can do the same thing i.e. instead of using strcmp

iceaway Over a year ago

If you want to compare strings it is probably best to stick with the standard methods. You cannot compare the pointers to the strings themselves as they will be pointing do different addresses in memory.

iceaway · Accepted Answer · 2011-06-21 19:33:09Z

I wrote something that will find the duplicates in an arbitrary string array and print out the names. I can't tell from your question if you specifically needed the indexes from the original array. This version will sort the array and the original indexes are "lost". But if you only need to know the names which are duplicates, the following should work:

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

#define TRUE 1
#define FALSE 0

/* Compare function for quicksort. Shamelessly taken directly from linux man
 * page */
static int cmpstringp(const void *p1, const void *p2)
{
   return strcmp(* (char * const *) p1, * (char * const *) p2);
}

void find_duplicates(char *strings[], size_t size)
{
    size_t i, j, last_occurrence;
    int is_unique;

    for(i = 0; i < size; ++i)
    {
        /* This variable tells us if the current element is unique */
        is_unique = TRUE;
        for(j = i; j < size; ++j)
        {
            /* Make sure we aren't comparing the element to itself */
            if((j != i) && (strcmp(strings[i], strings[j]) == 0))
            {
                /* Since the array is sorted, we store the index of the last
                 * duplicate and continue our search from there.
                 */
                last_occurrence = j;
                is_unique = FALSE;
            }
        }

        /* The element was not unique, print the name */
        if(is_unique == FALSE)
        {
            printf("Duplicate: %s\n", strings[i]);
            /* Set the counter to the next element we must check for
                             * "uniqueness" */
            i = last_occurrence;
        }
    }
}

int main(int argc, char *argv[])
{
    /* Test array */
    char *test_array[] = { "test1", "test2", "test3", "test1", 
                                   "test4", "test1", "test4" };

    /* Sort it using quicksort to make the process of finding duplicates
     * easier
     */
    qsort(test_array, 7, sizeof(char*), cmpstringp);

    find_duplicates(test_array, 7);

    return 0;
}

chenpp · Accepted Answer · 2011-09-18 07:20:40Z

I think this part would be the root of the problem:

    while ((dit = readdir(dip)) != NULL)               
       {
           condition_var = dit->name;      
           a[i] = condition_var;  
                         i++;
       }

I am not sure about how readdir() works but I would guess that every time it will return a temporal structure pointer to dit. a[i] saves condition_var as the pointer to the name. It is possible that later another call to readdir() is made and uses the exact the same mem space to return the temporal structure, overwriting the mem space that a previous condition_var pointing to.

I would suggest a safer way to do this part, which may be a little more complex, though.

    while ((dit = readdir(dip)) != NULL)               
       {
           int len = strlen(dit->name);
           condition_var = malloc(len+1);
           memncpy(condition_var, dit->name, len+1);
           //condition_var = dit->name;      
           a[i] = condition_var;  
                         i++;
       }

Remember to free every mem block that you malloced, at the end of the program. (Or you can just leave all these to OS to collect the mem--not recommended). The code should be like this:

     int j; // use another loop indicator if you already declared j for other uses.
     for(j = 0; j<i; j++)   // i is the length of a[].
     {
         free(a[j]);   
         // no worry about a[j] being NULL. free() does nothing on a NULL pointer.
         a[j] = NULL;
     }

Collectives™ on Stack Overflow

finding duplicates from the array of pointers

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related