problem passing pointer array

[pereges]

Hi, can some one please tell me why this program is not able to
function properly. I have a array a and i am trying to create a
pointer array b which points to elements less than 40 in a.

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

void create_ptr_list(int *a, int ***b, int n, int *size_ptr)
{
int i;
*size_ptr = 0;

for (i = 0; i < n; i++)
{
if (a < 40)
(*size_ptr) ++;
}

*b = malloc(sizeof(int *) * (*size_ptr));
(*size_ptr) = 0;
for (i = 0; i < n; i++)
{
if (a < 40)
(*b)[*size_ptr++]= &a;
}

}

int main(void)
{
int a[] = { 5, -6, 45, -100, 20, -150, 160, 40, 0, 0, 1};
int **b;
int size;
int i;

create_ptr_list(a, &b, 10, &size);
for(i = 0; i <size; i++)
printf("%d\n", *(b));

return (0);
}

 

[vippstar]

Hi, can some one please tell me why this program is not able to
function properly. I have a array a and i am trying to create a
pointer array b which points to elements less than 40 in a.

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

void create_ptr_list(int *a, int ***b, int n, int *size_ptr)

Change int ***b to int **b.
Change the 'int **b' in your main() to int *b.

 

[pereges]

Change int ***b to int **b.
Change the 'int **b' in your main() to int *b.

Wouldn't int *b lead to an array instead of array of pointers ? I
needed array of pointers hence int **b.

 

[pereges]

I would write that this way:

/* BEGIN new.c */

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

int *create_ptr_list(int *a, size_t n, size_t *size_ptr)
{
int *b;
size_t i;

*size_ptr = 0;
for (i = 0; n > i; ++i) {
if (40 > a) {
++*size_ptr;
}
}
b = malloc(*size_ptr * sizeof *b);
*size_ptr = 0;
if (b != NULL) {
for (i = 0; n > i; i++) {
if (40 > a) {
b[(*size_ptr)++]= a;
}
}
}
return b;

}

int main(void)
{
int a[] = {5, -6, 45, -100, 20, -150, 160, 40, 0, 0, 1};
int *b;
size_t size;
size_t i;

b = create_ptr_list(a, 10, &size);
if (b != NULL) {
for (i = 0; size > i; ++i) {
printf("%d\n", b);
}
} else {
puts("b == NULL");
}
free(b);
return 0;

}

/* END new.c */

I actually need an array of pointers. The array b will contains
pointers to elements in a which are less than 40.

The distinction between counters and sortable data
is more obvious with size_t counters.
I don't like to read sorting functions
where everything is int.

Sometimes using size_t or any unsigned entity can cause trouble in
sorting algorithms depending on how we write the sorting algorithm.
eg. just check the post i made on a quick sort algo today.

 

[vippstar]

Wouldn't int *b lead to an array instead of array of pointers ? I
needed array of pointers hence int **b.

Sorry, I just realized that.
The problem in your original code is in this line:

(*b)[*size_ptr++]= &a;

that increments 'size_ptr' as a pointer, not the value it points to.
Change it to (*b)[(*size_ptr)++] = &a;

 

[pereges]

Sorry, I just realized that.
The problem in your original code is in this line:> (*b)[*size_ptr++]= &a;

that increments 'size_ptr' as a pointer, not the value it points to.
Change it to (*b)[(*size_ptr)++] = &a;

Thanks, that solved it. I realize it was a stupid mistake

 

[vippstar]

On Jun 30, 8:59 pm, (e-mail address removed) wrote:

Wouldn't int *b lead to an array instead of array of pointers ? I
needed array of pointers hence int **b.

Sorry, I just realized that.
The problem in your original code is in this line:> (*b)[*size_ptr++]= &a;

that increments 'size_ptr' as a pointer, not the value it points to.
Change it to (*b)[(*size_ptr)++] = &a;

You also don't check the return value of malloc. It could be NULL, in
which case you just return; the caller can check for the
successfulness of create_ptr_list(a, &b, c, &d); with if(b !=
NULL) ...

Also the design is flawed, mainly because this works only for arrays
of ints, and because 40 is hardcoded, how about you change this to
struct vector { size_t nmemb, size; void *elements; };
struct vector *remove_if(const struct vector *v, int (*remove)(void
*));

though I don't like the 'remove_if' name, I chose it because there's a
similar function in common lisp named REMOVE-IF.

 

[pereges]

You also don't check the return value of malloc. It could be NULL, in
which case you just return; the caller can check for the
successfulness of create_ptr_list(a, &b, c, &d); with if(b !=
NULL) ...

Also the design is flawed, mainly because this works only for arrays
of ints, and because 40 is hardcoded, how about you change this to
struct vector { size_t nmemb, size; void *elements; };
struct vector *remove_if(const struct vector *v, int (*remove)(void
*));

though I don't like the 'remove_if' name, I chose it because there's a
similar function in common lisp named REMOVE-IF.

Well, I wrote the function just as a test function to understand about
array of pointers. Just chose int to make things simple.

 

[Chris Torek]

[warning: all my code here is untested]

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

int **create_ptr_list(int *a, size_t n, size_t *size_ptr)
{
int **b;
size_t i;

*size_ptr = 0;
for (i = 0; n > i; ++i) {
if (40 > a) {
++*size_ptr;
}
}
b = malloc(*size_ptr * sizeof *b);
*size_ptr = 0;
if (b != NULL) {
for (i = 0; n > i; i++) {
if (40 > a) {
b[(*size_ptr)++]= a + i;
}
}
}
return b;
}

I never really understand what people have against local variables

I would begin by rewriting the above function as:

/*
* Create an array of pointers to elements that exceed the
* specified limit. The input array is a, where 0 <= i < n,
* and the limit is the given value. The size of the pointer
* array is placed in *size_ptr.
*/
int **create_ptr_list(int *a, size_t n, int limit, size_t *size_ptr) {
int **b;
size_t i, over_limit;

for (i = over_limit = 0; i < n; i++)
if (a > limit)
over_limit++;
b = malloc(over_limit * sizeof *b);
if (b != NULL) {
for (i = over_limit = 0; i < n; i++)
if (a > limit)
b[over_limit++] = &a;
*size_ptr = over_limit;
} else {
/* optionally: handle error here if over_limit > 0 */
*size_ptr = 0;
}
return b;
}

More generally, one might pass the function a pointer to another
function that selects elements that pass some criterion or
criteria. This does make things substantially more complicated,
and possibly significantly slower (depending on how long it takes
to call functions):

int **create_ptr_list(int *a,
size_t n,
int (*match)(int, void *),
void *context,
size_t *size_ptr) {
int **b;
size_t i, nmatch;

for (i = nmatch = 0; i < n; i++)
if (match(a, context))
nmatch++;
b = malloc(nmatch * sizeof *b);
... the rest is obvious ...
}

We can get also rid of the "int" restriction by changing "int *a"
to "void *base" and adding a "size" parameter. Now, however, we
must build an array of "void *"s, so this may be going too far
(and indeed the match-with-context may already be too far). This
time I will verify that the match function does not change its
mind though:

void **create_ptr_list(void *base0, size_t n, size_t size,
int (*match)(void *, void *),
void *context,
size_t *size_ptr) {
unsigned char *base = base0;
void **b;
size_t i, nmatch1, nmatch2;

for (base = base0, i = nmatch1 = 0; i < n; base += size, i++)
if (match(base, context))
nmatch1++;
b = malloc(nmatch1 * sizeof *b);
if (b != NULL) {
for (base = base0, i = nmatch2 = 0; i < n; base += size, i++) {
if (match(base, context) {
if (nmatch2 >= nmatch1)
... handle error ...
b[nmatch2++] = base;
}
}
... optionally, make sure nmatch2 == nmatch1 ...
*size_ptr = nmatch2;
} else {
/* optionally: handle error here if over_limit > 0 */
*size_ptr = 0;
}
return b;
}

int main(void)
{
int a[] = {5, -6, 45, -100, 20, -150, 160, 40, 0, 0, 1};
int **b;
size_t size;
size_t i;

b = create_ptr_list(a, 10, &size);

Seems like this should be:

b = create_ptr_list(a, sizeof a / sizeof a[0], &size);

if (b != NULL) {
for (i = 0; size > i; ++i) {
printf("%d\n", *b);
}
} else {
puts("b == NULL");
}
free(b);
return 0;
}

 

[Nick Keighley]

int main(void)
{
   int a[] = { 5, -6, 45, -100, 20, -150, 160, 40, 0, 0, 1};

<snip>

there are 11 elements in this array

    create_ptr_list(a, &b, 10, &size);

you pass 10 to this function. Is this correct?

I use a macro like this

#define ARRAY_SIZE(A) (sizeof(A)/sizeof(*A))

(oops! another use for #define!)

<snip>