Re: weird problem



atv <alef@xxxxxxxxx> wrote:
i have a function called assign_coordinate. usually, i pass a malloced
pointer to it, then individual pointers are malloced within that function.
N problem here.

Now i have a situation in this software, where i need to assign new
coordinates to all nodes in the list.

The struct looks something like this:

Code a bit re-indented to make it a bit mroe readable.

struct test {
other variables;
char *x;
char *y;
char *z;
struct test *next;
struct test *prev;
}

So i first free the pointers x,y,z like so:

while(tmp != NULL) {
free(tmp->x);
free(tmp->y);
free(tmp->z);

Then call assign_coordinate:

assign_coordinate(tmp);

(Note that tmp itself is NOT free, only x,y,z pointers)
and so on:

tmp = tmp->next;
}


assign_coordinate looks like this:

GLvoid assign_coordinate(objects *tmp)

What's 'objects' for a kind of type? You don't show the typedef you
obviously must be using. 'GLvoid' is probably only a fanvy name for
'void', isn't it?

{
objects *search = o_head;

What's 'o_head'?

if ( ! tmp->prev && object_count == 1 )

Where is 'object_count' defined and what's its type?

srand((unsigned int)time(NULL));

/* Assign coordinate planes */
if ((tmp->x = malloc(sizeof(float) + 7)) == NULL) {

What is using "sizeof(float)" in the calculation of how much memory you
need when printing a float or double value out in ASCII good for? This
doesn't make any sense and is probably dangerous since sizeof(float)
can differ between different machines while the ASCII string you want
to output stays the same.

fprintf(stderr, "assign_coordinate: Could not allocate memory for x plane\n");
perror("malloc");

Using perror() here is useless since malloc() doesn't set set errno if
it fails to obtain as much memory as requested. perror() is meant to
print out a human-readable string for what errno is set to. And if
there's nothing that woul have set errno what it prints out is com-
pletely bogus.

exit(1);
}

snprintf(o_tail->x, sizeof(float) + 7, "%.0f",
((grid_size - grid_start) * rand()) / RAND_MAX +
grid_start);

Where is 'o_tail' defined? Are you sure you don't intend to write into
the string 'tmp->x' you just allocated before? And where are 'grid_start'
and 'grid_size' defined what what type do the have? Are they floats or
doubles or something else?

if ((tmp->y = malloc(sizeof(float) + 7)) == NULL) {
fprintf(stderr,"assign_coordinate: Could not allocate memory "
"for y plane\n");
perror("malloc");
exit(1);
}

snprintf(o_tail->y, sizeof(float) + 7, "%.0f", grid_object );

Where's the definition of grid_object?

if ((tmp->z = malloc(sizeof(float) + 7)) == NULL ) {
fprintf(stderr,"assign_coordinate: Could not allocate "
memory for z plane\n");
perror("malloc");
exit(1);
}

snprintf(o_tail->z, sizeof(float) + 7,"%.0f",
((grid_size - grid_start) * rand()) / RAND_MAX +
grid_start );

/* Check for possible collision */
while (search != NULL) {
if (collisions > 0 &&
collisions ==
((grid_size - grid_start) * (grid_size - grid_start))) {

Where is 'collisions' defined and what type does it have? Looks like an
integer counter, but you seem to compare it to some float or double value,
which, if these assumptions should be true, is rather dubious to say the
least. But since I have no idea what it's meant to test I can't make any
suggestions on how to correct it.

fprintf(stderr,"assign_coordinate: Sector collisions "
"100%%.Collisions: %d - Objects: %d - Grid size: "
"%.0f [%.0f^2 sectors]\n",
collisions, object_count,
(grid_size - grid_start) * (grid_size - grid_start),
grid_size - grid_start);
perror("assign_coordinate");

Again, using perror() is useless if nothing happened that would have set
errno.

exit(1);
}

if (tmp != search &&
!strcmp(tmp->x, search->x) &&
!strcmp(tmp->y, search->y) &&
!strcmp(tmp->z, search->z))
{
fprintf(stderr,"%s %s %s %s - %s %s %s %s\n",
tmp->hostname, tmp->x, tmp->y, tmp->z,
search->hostname, search->x, search->y, search->z);
collisions++;
free(tmp->x);
free(tmp->y);
free(tmp->z);
assign_coordinate(tmp);
}

search=search->next;
}
}

I have no snprintf check for error in there yet, but i checked with
printf statements if it
assigned properly, and it did.

What did you printf() out? E.g. 'tmp->x' would point to uninitialized
memory, while printing out 'o_tail->x' probably won't tell you anything
about 'tmp->x' (unless you're hinding some code where you assign 'tmp'
to 'o_tail').

But, when going in the search loop,
apparently tmp->x y and z and search->x,y,z are not valid anymore
because they are both NULL so it seems. That's
why i get 100% collisions all the time, or so i gather. I don't
understand, because i'm
excluding myself (and thus my freed x,y,z) and i did not yet free
future nodes x,y,z in the list.

Sorry, but nothing you write here makes much sense. How do you deter-
mine that tmp->x or tmp->y or tmp->z are NULL? And since you don't
show what 'search' is and how it got set up there's no way someone
could figure out what's going wrong. If you post code it's best if
it's a complete, compilable program. If that is really not possible
take care to include definition of the variables being used and at
least give a hint what they are set to. And only post the code you
were actually using, not something you copied manually.

Regards, Jens
--
\ Jens Thoms Toerring ___ jt@xxxxxxxxxxx
\__________________________ http://toerring.de
.



Relevant Pages

  • Re: 32 or 64 bit processor info in C
    ... successful allocation of buff3 could obscure why buff2's allocation ... malloc() need not set errno and some do not, ... There is not enough memory available to allocate size ...
    (comp.lang.c)
  • Re: maximum size of a hash table
    ... > code results in Ohash look ups because there is a lot of data in the ... that results in fewer collisions. ... buckets as there are bytes of memory. ... of memory, so that's a theoretical limit that can't be reached in practice. ...
    (comp.lang.perl.misc)