bsearch script segfault

  • Thread starter Ramprasad A Padmanabhan
  • Start date
R

Ramprasad A Padmanabhan

I have got a pretty simple script , that uses bsearch to look for a
particular element

The problem is , it simply segfaults inside the compare function.

I have a similar script that works fine , and now this simply segfaults.
I am driving myself nuts , last 4 hrs , why such a simple thing would
not work

Pls someone help
Thanks
Ram



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

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;
return(strcmp(u1->a_id,(*u2)->a_id));
}

struct aliasid* new_aliasid(char* id,int numids) {
struct aliasid* a;
a = malloc(sizeof(struct aliasid) +10);
strcpy(a->a_id,id);
a->count = 0;
return(a);
}

int main(int argc,char* argv[]){
struct aliasid **aliases , **tmp1;
char the_string[100];

aliases = malloc(3 * sizeof(void*));
if(aliases == NULL ) {
printf("malloc failed");
exit(1);
}
aliases[0]= new_aliasid("abc",3);
aliases[1]= new_aliasid("def",3);
aliases[2]=NULL;
tmp1 = (struct aliasid **) bsearch("abc",aliases,2,sizeof( struct
aliasid),compare_aliases);

if(tmp1 == NULL){
printf("The id %s was not found\n",the_string);
} else {
printf("The id %s was found\n",the_string);

}
return(0);
}
 
R

Ramprasad A Padmanabhan

Ramprasad said:
I have got a pretty simple script , that uses bsearch to look for a
particular element

The problem is , it simply segfaults inside the compare function.

I have a similar script that works fine , and now this simply segfaults.
I am driving myself nuts , last 4 hrs , why such a simple thing would
not work

Pls someone help
Thanks
Ram



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

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;
return(strcmp(u1->a_id,(*u2)->a_id));
}

struct aliasid* new_aliasid(char* id,int numids) {
struct aliasid* a;
a = malloc(sizeof(struct aliasid) +10);
strcpy(a->a_id,id);
a->count = 0;
return(a);
}

int main(int argc,char* argv[]){
struct aliasid **aliases , **tmp1;
char the_string[100];

aliases = malloc(3 * sizeof(void*));
if(aliases == NULL ) {
printf("malloc failed");
exit(1);
}
aliases[0]= new_aliasid("abc",3);
aliases[1]= new_aliasid("def",3);
aliases[2]=NULL;
tmp1 = (struct aliasid **) bsearch("abc",aliases,2,sizeof( struct
aliasid),compare_aliases);

if(tmp1 == NULL){
printf("The id %s was not found\n",the_string);
} else {
printf("The id %s was found\n",the_string);

}
return(0);
}


Ok , I think I have got the problem ,

If I declare aliases as
*aliases[3];
Then this works fine




So does that mean I cannot dynamically allocate memory to a structure
array Which I use in bsearch

Thanks
Ram
 
J

Jens.Toerring

Ramprasad A Padmanabhan said:
I have got a pretty simple script , that uses bsearch to look for a
particular element
The problem is , it simply segfaults inside the compare function.
I have a similar script that works fine , and now this simply segfaults.
I am driving myself nuts , last 4 hrs , why such a simple thing would
not work
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;

Why do you have p2 being a pointer to a pointer? That doesn't
make much sense.
return(strcmp(u1->a_id,(*u2)->a_id));
}
struct aliasid* new_aliasid(char* id,int numids) {
struct aliasid* a;
a = malloc(sizeof(struct aliasid) +10);

Why do you allocate 10 more bytes than you need? And things would be a
lot easer to maintain (and to read) if you would write this as

a = malloc( sizeof *a + 10 );
strcpy(a->a_id,id);
a->count = 0;
return(a);
}
int main(int argc,char* argv[]){
struct aliasid **aliases , **tmp1;
char the_string[100];
aliases = malloc(3 * sizeof(void*));

Why sizeof(void*)? You obviously want a array of structure pointers.
Just write it as

aliases = malloc( 3 * sizeof *aliases );
if(aliases == NULL ) {
printf("malloc failed");
exit(1);
}
aliases[0]= new_aliasid("abc",3);
aliases[1]= new_aliasid("def",3);
aliases[2]=NULL;
tmp1 = (struct aliasid **) bsearch("abc",aliases,2,sizeof( struct
aliasid),compare_aliases);

What's definitely wrong here is the 'size' argument, i.e. the fourth
argument: what you pass to bsearch() is an array *of pointers*, not
of structures. That's the most likely reason for the crash you're
seeing. Make that 'sizeof *aliases' and you should be fine.

And you don't need the cast, like for malloc() it will only keep the
compiler from complaining if you forgot to include said:
if(tmp1 == NULL){
printf("The id %s was not found\n",the_string);
} else {
printf("The id %s was found\n",the_string);

'the_string' never has been assigned a value, so you can't print it.
}
return(0);

Shouldn't you clean up after yourself and free the memory you allocated;-)
Regards, Jens
 
C

Christopher Benson-Manica

Ramprasad A Padmanabhan said:
I have got a pretty simple script , that uses bsearch to look for a

<pedantry>A C program isn't technically a "script," is it?</pedantry>
 
A

Anupam

Ramprasad A Padmanabhan said:
Ramprasad said:
I have got a pretty simple script , that uses bsearch to look for a
particular element

The problem is , it simply segfaults inside the compare function.

I have a similar script that works fine , and now this simply segfaults.
I am driving myself nuts , last 4 hrs , why such a simple thing would
not work

Pls someone help
Thanks
Ram



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

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;
return(strcmp(u1->a_id,(*u2)->a_id));
}

struct aliasid* new_aliasid(char* id,int numids) {
struct aliasid* a;
a = malloc(sizeof(struct aliasid) +10);
strcpy(a->a_id,id);
a->count = 0;
return(a);
}

int main(int argc,char* argv[]){
struct aliasid **aliases , **tmp1;
char the_string[100];

aliases = malloc(3 * sizeof(void*));
if(aliases == NULL ) {
printf("malloc failed");
exit(1);
}
aliases[0]= new_aliasid("abc",3);
aliases[1]= new_aliasid("def",3);
aliases[2]=NULL;
tmp1 = (struct aliasid **) bsearch("abc",aliases,2,sizeof( struct
aliasid),compare_aliases);

if(tmp1 == NULL){
printf("The id %s was not found\n",the_string);
} else {
printf("The id %s was found\n",the_string);

} return(0);
}


Ok , I think I have got the problem ,

If I declare aliases as
*aliases[3];
Then this works fine




So does that mean I cannot dynamically allocate memory to a structure
array Which I use in bsearch

No it does not mean that. The problem is that you are accessing aliases[1] etc.
which is a strict no no without allocating memory for the array.
Do this :
aliases=malloc(sizeof *aliases *3); first

( PS : or malloc(sizeof* aliases*3); ;) as discussed in another thread)
Thanks
Ram

PS Do also look at what Jens had to say. All of it applies too.
Regards,
Anupam
 
R

Ramprasad A Padmanabhan

Ramprasad A Padmanabhan said:
I have got a pretty simple script , that uses bsearch to look for a
particular element

The problem is , it simply segfaults inside the compare function.

I have a similar script that works fine , and now this simply segfaults.
I am driving myself nuts , last 4 hrs , why such a simple thing would
not work

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

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;


Why do you have p2 being a pointer to a pointer? That doesn't
make much sense.

return(strcmp(u1->a_id,(*u2)->a_id));
}

struct aliasid* new_aliasid(char* id,int numids) {
struct aliasid* a;
a = malloc(sizeof(struct aliasid) +10);


Why do you allocate 10 more bytes than you need? And things would be a
lot easer to maintain (and to read) if you would write this as

a = malloc( sizeof *a + 10 );

strcpy(a->a_id,id);
a->count = 0;
return(a);
}

int main(int argc,char* argv[]){
struct aliasid **aliases , **tmp1;
char the_string[100];

aliases = malloc(3 * sizeof(void*));


Why sizeof(void*)? You obviously want a array of structure pointers.
Just write it as

aliases = malloc( 3 * sizeof *aliases );

if(aliases == NULL ) {
printf("malloc failed");
exit(1);
}
aliases[0]= new_aliasid("abc",3);
aliases[1]= new_aliasid("def",3);
aliases[2]=NULL;
tmp1 = (struct aliasid **) bsearch("abc",aliases,2,sizeof( struct
aliasid),compare_aliases);


What's definitely wrong here is the 'size' argument, i.e. the fourth
argument: what you pass to bsearch() is an array *of pointers*, not
of structures. That's the most likely reason for the crash you're
seeing. Make that 'sizeof *aliases' and you should be fine.

Thanks that was it. This is where I was making a mistake.
I still have a lot of doubts , I hope people in this group wont mind

The problem with my C skills is that it is a lot of hit and try. I am
not the bookish type , who would go thru the theory first and then
implement it nor Have I had any training.

I was doing
aliases = malloc(3 * sizeof(void*)); instead of
aliases = malloc(3 * sizeof *aliases);

But does it make a difference.
Is not void* too a pointer variable as much as *aliases , and so would
have the same size


Thanks
Ram
 
A

Arthur J. O'Dwyer

Ramprasad A Padmanabhan said:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;

Why do you have p2 being a pointer to a pointer? That doesn't
make much sense.

This may have been pointed out already (it should've been),
but yes, the OP's code is almost correct. The way he's writing
it, p1 is a pointer to string literal ("abc") and p2 is the address
of a pointer to 'struct aliasid'. So the levels of indirection
match up -- but he's playing with fire when it comes to the types
involved. p1 does *not* point to a 'struct aliasid' -- it points
to a 'char', the first character in the string literal.
Also, the OP is casting away constness. Don't cast -- it's
really not necessary, with *very* few exceptions, and it clutters
up the code something awful.

The whole function should thus read

int compare_aliases(const void *p1, const void *p2)
{
char const *u1 = p1;
struct aliasid * const *u2 = p2;
return strcmp(u1, (*u2)->a_id);
}

(In C99, this is legal. In C89, maybe not -- language lawyers
to the rescue!? Someone else can explain the traditional way
to do it, passing a full-fledged key object and so on.)
Thanks that was it. This is where I was making a mistake.
I still have a lot of doubts , I hope people in this group wont mind

Go ahead. [Just call the things you have "questions," though,
because that's probably what they are. Do they end with question
marks, like this? Then they're questions.]

I was doing
aliases = malloc(3 * sizeof(void*)); instead of
aliases = malloc(3 * sizeof *aliases);

But does it make a difference.
Is not void* too a pointer variable as much as *aliases , and so would
have the same size

(void *) is a pointer type, yes, but it's not the same type as
(struct aliasid *), which is what the members of the array 'aliases'
are. Again with few exceptions, types that aren't the same type
don't usually have to be the same size.
The advantage of
aliases = malloc(3 * sizeof *aliases); over
aliases = malloc(3 * sizeof (struct aliasid *));

is that the former is shorter, less cluttered, and doesn't contain
the names of any types. So when you decide that 'aliasid' is a
confusing name for the structure and make it 'MyLib_alias' instead,
you have one fewer place that needs changing. :) Also, and less
contrivedly, note that both of these
aliases = malloc(3 * sizeof (struct aliasid));
aliases = malloc(3 * sizeof (struct aliasid **));

are wrong, wrong, wrong! Using the 'sizeof *aliases' construction
simply prevents this sort of hard-to-find mistake from happening,
as long as you follow the simple "foo = malloc(X * sizeof *foo)"
rule everywhere in your code. Mistakes stand out.

-Arthur
 
J

Jens.Toerring

Arthur J. O'Dwyer said:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;

Why do you have p2 being a pointer to a pointer? That doesn't
make much sense.
This may have been pointed out already (it should've been),
but yes, the OP's code is almost correct. The way he's writing
it, p1 is a pointer to string literal ("abc") and p2 is the address
of a pointer to 'struct aliasid'. So the levels of indirection
match up -- but he's playing with fire when it comes to the types
involved. p1 does *not* point to a 'struct aliasid' -- it points
to a 'char', the first character in the string literal.
Also, the OP is casting away constness. Don't cast -- it's
really not necessary, with *very* few exceptions, and it clutters
up the code something awful.

The whole function should thus read
int compare_aliases(const void *p1, const void *p2)
{
char const *u1 = p1;
struct aliasid * const *u2 = p2;
return strcmp(u1, (*u2)->a_id);
}

I might be misunderstanding something here, but as far as I can see
the compare_aliases() function gets two pointers pointing to objects
of exactly the same type, struct aliasid. And while the original
version of the function might work (as long the sizes of pointers
and pointers to pointers are identical), it's at least misleading.
As far as I can see it should be

int compare_aliases( const void *p1, const void *p2 )
{
const struct aliasid *u1 = p1;
const struct aliasid *u2 = p2;
return strcmp( u1->a_id, u2->a_id );
}

Regards, Jens
 
A

August Derleth

<pedantry>A C program isn't technically a "script," is it?</pedantry>

A script is whatever the author calls a script, really. There's no hard
and fast rule about these things.

Compilation and interpretation are not properties of the language, merely
properties of the language's environment. C is usually compiled, but it
would be trivial* to create a C interpreter (which has, in fact, been
done). Perl is nearly always interpreted, but there exist mechanisms to
compile it down to C, which is then usually compiled down to native
machine code. (Admittedly, the code usually sucks, but it is there and it
works.)

*Well, easier than making an interpreter for a language as complex as
Perl.

So, if interpretation/compilation isn't a factor, is complexity? Again,
not really: How complexity is defined is nearly always dependent upon the
language. What would be complex in C (say, an efficient regex-driven text
parser) would be trivial in Perl, and what would be trivial in C (say, an
efficient interrupt handler for an embedded system) would be next to
impossible in Perl. A complex assembly language program, requiring
multiple man-years to plan, code, test, and debug, could be accomplished
in a few hours in a high-level language, especially if that language is
domain-specific (think SQL, or APL).
 
J

Jens.Toerring

I might be misunderstanding something here, but as far as I can see
the compare_aliases() function gets two pointers pointing to objects
of exactly the same type, struct aliasid. And while the original

Sorry, I just realized I *was* misunderstanding something badly,
confusing the comparison functions used e.g. with qsort() and the
one needed for bsearch(). Please forget about my post.

Regards, Jens
 
R

Ramprasad A Padmanabhan

Arthur said:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

struct aliasid {
char a_id[100];
int count;
};
int compare_aliases(const void *p1, const void *p2 ){
const struct aliasid *u1 = (const struct aliasid *) p1;
const struct aliasid **u2 = (const struct aliasid **) p2;

Why do you have p2 being a pointer to a pointer? That doesn't
make much sense.


This may have been pointed out already (it should've been),
but yes, the OP's code is almost correct. The way he's writing
it, p1 is a pointer to string literal ("abc") and p2 is the address
of a pointer to 'struct aliasid'. So the levels of indirection
match up -- but he's playing with fire when it comes to the types
involved. p1 does *not* point to a 'struct aliasid' -- it points
to a 'char', the first character in the string literal.
Also, the OP is casting away constness. Don't cast -- it's
really not necessary, with *very* few exceptions, and it clutters
up the code something awful.



The whole function should thus read

int compare_aliases(const void *p1, const void *p2)
{
char const *u1 = p1;
struct aliasid * const *u2 = p2;
return strcmp(u1, (*u2)->a_id);
}

(In C99, this is legal. In C89, maybe not -- language lawyers
to the rescue!? Someone else can explain the traditional way
to do it, passing a full-fledged key object and so on.)

How do you explain
struct aliasid * const *u2 = p2;
Even though that works perfect, I havent understood this.

When I did
const struct aliasid **u2 = p2;
without the cast my compiler (gcc on linux) , gives a warning . But with
your way of writing it does not. But I dont see any difference.
(void *) is a pointer type, yes, but it's not the same type as
(struct aliasid *), which is what the members of the array 'aliases'
are. Again with few exceptions, types that aren't the same type
don't usually have to be the same size.

Why ? Are not pointers , simply variables that hold the beginning
address of structure. So the addresses are always going to be similar (
I mean same sized ) wether the indicate beginning of one structure or
another or a char .

I am basicallay a perl programmer , Where a refernce is just a simple
scalar variable. No matter what it points to. and they are all same.
(Or atleast thats what I had assumed till now )

are wrong, wrong, wrong! Using the 'sizeof *aliases' construction
simply prevents this sort of hard-to-find mistake from happening,
as long as you follow the simple "foo = malloc(X * sizeof *foo)"
rule everywhere in your code. Mistakes stand out.

Honest, I did not know this. I will be more careful from now.

Thanks
Ram

Thanks
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top