How to find an element in an array of structs in C?

Question

I have to write a function that finds a product with given code from the given array. If product is found, a pointer to the corresponding array element is returned.

My main problem is that the given code should first be truncated to seven characters and only after that compared with array elements.

Would greatly appreciate your help.

struct product *find_product(struct product_array *pa, const char *code) 

{   
char *temp;
int i = 0;
    while (*code) {
        temp[i] = (*code);
        code++;
        i++;
        if (i == 7)
            break;
    }
    temp[i] = '\0';

for (int j = 0; j < pa->count; j++)
    if (pa->arr[j].code == temp[i])
        return &(pa->arr[j]);
}

First thing I noticed was temp is not initialized and you are using it which will lead to undefined behavior — Gopi
– Gopi, Commented Apr 9, 2015 at 7:38
You did not allocate any memory for temp. Use malloc/calloc to do it. And what did you mean by "given code should first be truncated to seven characters"? — Spikatrix
– Spikatrix, Commented Apr 9, 2015 at 7:39
if (pa->arr[j].code == temp[i]) is the same as if (pa->arr[j].code == 0). Use strcmp or strncmp to compare two strings — Spikatrix
– Spikatrix, Commented Apr 9, 2015 at 7:40
You can use strncpy() to copy the first 7 characters of code to temp, you don't need to write your own loop. — Barmar
– Barmar, Commented Apr 9, 2015 at 7:41

michaelb · Accepted Answer · 2015-04-09 07:49:26Z

10

Why don't you just use strncmp in a loop?

struct product *find_product(struct product_array *pa, const char *code) 
{ 
   for (size_t i = 0; i < pa->count; ++i)
   {
      if (strncmp(pa->arr[i].code, code, 7) == 0)
          return &pa->arr[i];
   }
   return 0;
}

answered Apr 9, 2015 at 7:49

michaelb

4512 silver badges11 bronze badges

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

Comments

Gopi · Accepted Answer · 2015-04-09 08:31:11Z

4

temp is a pointer which is uninitialized and you are dereferencing it which will lead to undefined behavior.

temp = malloc(size); // Allocate some memory size = 8 in your case

One more mistake I see is

if (pa->arr[j].code == temp[i]) // i is already indexing `\0`

should be

strcmp(pa->arr[j].code,temp); // returns 0 if both the strings are same

This code can completely be avoided if you can use strncmp()

edited Apr 9, 2015 at 8:31

answered Apr 9, 2015 at 7:42

Gopi

19.9k4 gold badges28 silver badges37 bronze badges

Comments

David Ranieri · Accepted Answer · 2015-04-09 08:10:55Z

2

As pointed out by others, you are using temp uninitialized and you are always comparing characters with '\0'.

You don't need a temp variable:

int strncmp ( const char * str1, const char * str2, size_t num );

Compare characters of two strings

Compares up to num characters of the C string str1 to those of the C string str2.

/* Don't use magic numbers like 7 in the body of function */
#define PRODUCT_CODE_LEN 7

struct product *find_product(struct product_array *pa, const char *code) 
{   
    for (int i = 0; i < pa->count; i++) {
        if (strncmp(pa->arr[i].code, code, PRODUCT_CODE_LEN) == 0)
            return &(pa->arr[i]);
    }
    return NULL; /* Not found */
}

edited Apr 9, 2015 at 8:10

answered Apr 9, 2015 at 7:51

David Ranieri

41.2k8 gold badges58 silver badges103 bronze badges

1 Comment

Quentin Over a year ago

UB galore if the product is not to be found.

AndersK · Accepted Answer · 2015-04-09 08:22:56Z

When you write char* temp; you are just declaring an uninitialized pointer

In your case since you say that the code is truncated to 7 you could create a buffer on the stack with place for the code

char temp[8];

Writing

temp[i] = (*code);
code++;
i++;

Can be simplified to:

temp[i++] = *code++;

In your loop

for (int j = 0; j < pa->count; j++)
    if (pa->arr[j].code == temp[i])
        return &(pa->arr[j]);

You are comparing the address of code and the character value of temp[i] which incidentally could be 8 and outside the array.

Instead what you want to do is compare what code points to and what temp contains:

for (int j = 0; j < pa->count; j++)
    if (!strncmp(pa->arr[j].code, temp, 7)
        return &(pa->arr[j]);

You should also return NULL; if nothing was found, seems you do not return anything.

Probably a good thing is also to make sure your temp[] always contains 7 characters.

Collectives™ on Stack Overflow

How to find an element in an array of structs in C?

4 Answers 4

Comments

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related