weird problem

[atv]

at least to me it is. I can't figure out for the life what it is i'm
doing wrong here.

i have a function called assign_coordinate. usually, i pass a malloced pointer
to it, then individual pointers are malloced within that function. No
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:

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)
{
objects *search=o_head;

if(!tmp->prev&&object_count==1)
srand((unsigned int)time(NULL));

/* Assign coordinate planes */
if((tmp->x=malloc(sizeof(float)+7))==NULL) {
fprintf(stderr,"assign_coordinate: Could not allocate memory for x plane\n");
perror("malloc");
exit(1);}
snprintf(o_tail->x,sizeof(float)+7,"%.0f",((grid_size-grid_start)*rand())/RAND_MAX+grid_start);
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);
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)))
{
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");
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. 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.

Thanks for any help.
Rgds,
-alef

 

[Jens Thoms Toerring]

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:

(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

 

[Alef.Veld]

obviously must be using. 'GLvoid' is probably only a fanvy name for
'void', isn't it?

GL is a prefix for using variables with GLUT. Yes they are the same.
objects is a typedef for a struct like:

typedef struct objects {
char *x;
char *y;
char *z;
} objects;


o_head is a objects pointer pointing to the first node in the linked
list i'm using.

object_count is a global (declared extern in header and intialized in
other function). type is int.

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.

I know this, i will just remove the sizeof float and put the amount of
bytes in there i need. I still need to change this, didn't get around
to it yet.

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.

Hmm, yes kind of a shame malloc doesn;t use errno. I use perror because
i want to do consistent error reporting so i use it with every error,
if errno is used or not.

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?

o_tail is a global pointing to the last node in the linked list. I'm
not sure you mean by 'i don't intend to write into
the string tmp->x you just allocated before'. I want to write into the
string, i just freed them remember? grid_start
and grid_size are floats. they are set in the beginning of the program
to -10 and grid_size is calculated dynamically, depending on how many
objects there are.
float, value is 2.0

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.

collisions is int and defined as a global (and declared extern) . It is
only reset when the grid_size is increased, otherwise it is incremented
for every collision it matches.

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.

I know

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').

Eek

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.

search is just a tmp pointer set to the 'head' of the list. I don't
understand
why i would want to test x y and z for NULL. I free them before i dive
into
assign_coordinate, and allocate them right back after. So they are
always
allocated. The other pointers search encounters, are not yet freed and
still
valid.

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.

I'm using everything i posted, albeit somewhat simplified posted maybe.
I hope this clear things up, if not, i will try and post longer
listings of code with the variables
in there.

 

[Jens Thoms Toerring]

On Jan 24, 7:51 pm, (e-mail address removed) (Jens Thoms Toerring) wrote:

Yes they are the same. objects is a typedef for a struct like:

typedef struct objects {
char *x;
char *y;
char *z;
} objects;

Obviously plus a few missing members like prev, next and hostname;-)

o_head is a objects pointer pointing to the first node in the linked
list i'm using.

Well, you don't show us anything about this linked list and what
'tmp' has for a relation to it - 'tmp' could point to an element
of this list or to something completely different. All I can do
make guesses, e.g. that for some unknown reason you assume that
'tmp' magically becomes identical to 'o_tail'.

Hmm, yes kind of a shame malloc doesn;t use errno. I use perror because
i want to do consistent error reporting so i use it with every error,
if errno is used or not.

Well, malloc() doesn't do that, so you've got to deal with it;-)

o_tail is a global pointing to the last node in the linked list. I'm
not sure you mean by 'i don't intend to write into
the string tmp->x you just allocated before'. I want to write into the
string, i just freed them remember?

What did you just free()? On the contrary, you just _allocated_
memory for 'tmp->x'. And now you don't write into that shiny new
memory you just got but somewhere else, i.e. to what 'o_tail->x'
points to. And there is nothing to be seen that would indicate
that 'o_head->x' and 'tmp->x' are identical (that would probably
require that 'o_head' is identical to 'tmp', but there's nothing
that would clearly show that this is the case, it rather looks
like this isn't true)..

And worse, sometime later you use strcmp() on 'tmp->x', even
though you never initialized the memory 'tmp->x' is pointing
to. And since strcmp() isn't save to be used with uninitialized
memory this is badly broken.

grid_start and grid_size are floats. they are set in the beginning of
the program to -10 and grid_size is calculated dynamically, depending
on how many objects there are.
collisions is int and defined as a global (and declared extern) . It is
only reset when the grid_size is increased, otherwise it is incremented
for every collision it matches.

So I am lead to assume that '(grid_size - grid_start)' is the size of
a square grid and you want to compare the number of points on that
grid to the number of 'collisions'. In that case you need to round
the result of squaring the grid size to the nearest int, otherwise
you have a good chance that the comparison will never be true -
floating point numbers aren't necessarily exact, if grid_size is e.g.
5,0 and grid_start is -7.0, then the result of squaring the differ-
rence could easily be e.g. 143.999999999998, which is not equal to
144.

Look, here you suddenly use 'tmp->x' as if it would be pointing to
a string. But actually it points to uninitialized memory. Let's
hope that at least 'search->x' points to a real string;-)

search is just a tmp pointer set to the 'head' of the list. I don't
understand
why i would want to test x y and z for NULL.

I was just asking how you came to that conclusion.

I free them before i dive into assign_coordinate, and allocate them
right back after. So they are always allocated. The other pointers
search encounters, are not yet freed and still valid.

That's exactly my point. You assign to the 'x', 'y' and 'z' members
of 'tmp' and make sure they are not NULL and you don't touch the
members of 'search', so, assuming they weren't NULL to start with,
they shouldn't suddenly be set NULL. All I asked was how you came
up with the idea that they would be NULL despite this;-)

Regards, Jens

 

[Alef.Veld]

Well, yes, the struct has somewhat more members but i didn't want to
bore you with those. They are not relevant in any case.

'tmp' has for a relation to it - 'tmp' could point to an element
of this list or to something completely different. All I can do
make guesses, e.g. that for some unknown reason you assume that
'tmp' magically becomes identical to 'o_tail'.

When i want to move through my list, i always declare a tmp *ptr, and
assign that either to the head or the tail of the list, or sometimes a
node somewhere specific.

memory for 'tmp->x'.

Hmm, maybe i wasn't clear. Before i call assign_coordinate and malloc
memory to x y and z, i free
them (outside of assign_coordinate). In my first post i mentioned:
--
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;}
--
So i freed those x y and z pointers in tmp.

And now you don't write into that shiny new

memory you just got but somewhere else, i.e. to what 'o_tail->x'
points to. And there is nothing to be seen that would indicate
that 'o_head->x' and 'tmp->x' are identical (that would probably
require that 'o_head' is identical to 'tmp', but there's nothing
that would clearly show that this is the case, it rather looks
like this isn't true)..

Sorry my fault, i did not supply you with enough code. What i am doing
is that after i resize my grid (or rather decrease), there is a fat
chance that my previous coordinates are not correct anymore. So i free
all my x, y and z pointers, as show previously, and call
assign_coordinate to create new ones, randomly. Then i search through
my existing list if there is a possible collision. Remember, i did not
yet free the other ones, only the first one. When this one gets new
coordinates, tmp goes to tmp->next, then calls free() again, etc.


..So I am lead to assume that '(grid_size - grid_start)' is the size of

a square grid and you want to compare the number of points on that
grid to the number of 'collisions'. In that case you need to round
the result of squaring the grid size to the nearest int,

You're right. This might actually be my problem then. Would a cast
suffice or do i have to manually
round this off somehow with a conversion specifier.
otherwise

you have a good chance that the comparison will never be true -
floating point numbers aren't necessarily exact, if grid_size is e.g.
5,0 and grid_start is -7.0, then the result of squaring the differ-
rence could easily be e.g. 143.999999999998, which is not equal to
144.

see my answer below
I don't test them for NULL because i do not search the same ptr i'm
supplying (which x ,y,z are freed) (although i did not specifically set
it to NULL i assuming it's valid because i allocated memory in
assign_coordinate.

of 'tmp' and make sure they are not NULL and you don't touch the
members of 'search', so, assuming they weren't NULL to start with,
they shouldn't suddenly be set NULL. All I asked was how you came
up with the idea that they would be NULL despite this;-)

Ah. I see that without proper code this is getting confusing for the
both of us . I will see if i can supply both 2 functions which cause
my troubles.

Regards, Jens

Many thanks,
-alef

 

[Jens Thoms Toerring]

You're right. This might actually be my problem then. Would a cast
suffice or do i have to manually round this off somehow with a
conversion specifier.

A conversion specifier only can be used in printf() etc. And a cast
alone won't do since it simply cuts off everything after the decimal
point. But that won't get you 144 from 143.99999999. You need to
round to the nearest integer, e.g. by using

( int ) ( d < 0.0 ? ceil( d - 0.5 ) : floor( d + 0.5 ) );

where d is your float or double - or, since you are rounding
the square of a float, which is always positive,

( int ) floor( d + 0.5 )

will do.

BTW, you also have an off-by-one (fence-post) error: if the size
of the grid is (a - b) then the number of points on th grid is
(a - b + 1) * (a - b + 1).

Ah. I see that without proper code this is getting confusing for the
both of us . I will see if i can supply both 2 functions which cause
my troubles.

Yes, please do that;-)
Regards, Jens

 

[Alef.Veld]

Hmm, i thought i had replied to this. Probably clicked on discard. Ok,
one more time:
The GL variables or functions are specific for glut/opengl, you can
discard these. I only provided the complete functions so you can see
them into context.

ng.h:

/* Grid options */
#define SECTOR_SIZE 1.0
#define GRID_SIZE 35.0
#define GRID_START -10.0
#define GRID_OBJECT 2.0

/* Main database structure for holding objects */
typedef struct objects {
/* Host names */
char *hostname;
char *ip_addr;
/* Host status */
char *status;
/* Host coordinates */
char *x;
char *y;
char *z;
/* SNMPv2-MIB */
char *sysDescr;
char *sysUpTime;
char *sysContact;
char *sysName;
char *sysLocation;
/* IP-MIB */
char *ipForwarding;
char *ipAdEntNetMask;
/* IF-MIB */
char *ifNumber;
struct objects *next;
struct objects *prev;
} objects;

/* Variables */
extern GLint collisions;
extern GLfloat sector_size;
extern GLfloat grid_size;
extern GLfloat grid_start;
extern GLfloat grid_object;
extern objects *o_head,*o_tail;
extern events *e_head,*e_tail;
extern GLint object_count,event_count;
extern GLint x_event,y_event;
extern GLfloat x,y,z;
extern GLfloat a,b,c,d,e,f;
extern GLsizei width,heigth;
extern GLfloat fps;
extern GLint dl_display_grid;

GLvoid display_grid(GLvoid);
GLvoid assign_coordinate(objects *tmp);

display_grid.c:

#include "ng.h"

GLfloat x,y,z;

GLfloat sector_size=SECTOR_SIZE;
GLfloat grid_size;
GLfloat grid_start=GRID_START;
GLfloat grid_object=GRID_OBJECT;

GLvoid display_grid(GLvoid)
{
objects *tmp=o_head;


/* Set initial value */
if(!object_count)
grid_size=GRID_SIZE;

/* Check here if we need to resize the grid */
if(((grid_size*grid_size)/object_count)<=2) {
grid_size+=1.0;
/* Reset collisions */
collisions=0; }

if(object_count!=0&&object_count!=1) {
if((grid_size*grid_size)/object_count>=2) {
if(collisions) {
if((grid_size*grid_size)/collisions>=2) {
grid_size-=1.0;
while(tmp!=NULL) {
free(tmp->x);
free(tmp->y);
free(tmp->z);
assign_coordinate(tmp);
tmp=tmp->next;}
}
}
else if(!collisions) {
grid_size-=1.0;
while(tmp!=NULL) {
free(tmp->x);
free(tmp->y);
free(tmp->z);
assign_coordinate(tmp);
tmp=tmp->next;}
}
}
}

glBegin(GL_QUADS);
for(x=grid_start;x<=grid_size;x+=sector_size) {
for(y=grid_start;y<=grid_size;y+=sector_size) {
/* Set color for QUADS */
glColor4f(0.0f,0.0f,0.2,1.0f);
/* Draw QUADS */
glVertex3f(x,y,0); /* Left bottom */
glVertex3f(x+sector_size,y,0); /* Right bottom */
glVertex3f(x+sector_size,y+sector_size,0); /* Right upper */
glVertex3f(x,y+sector_size,0); /* Left upper */
}
}
glEnd();

glBegin(GL_LINES);
for(x=grid_start;x<=grid_size;x+=sector_size) {
for(y=grid_start;y<=grid_size;y+=sector_size) {
/* Set color for LINES */
glColor4f(0.0f,0.2f,0.0,0.5f);
/* Draw lines */
glVertex3f(x,y,0);
glVertex3f(x+sector_size,y,0);
glVertex3f(x+sector_size,y,0);
glVertex3f(x+sector_size,y+sector_size,0);
glVertex3f(x+sector_size,y+sector_size,0);
glVertex3f(x,y+sector_size,0);
glVertex3f(x,y+sector_size,0);
glVertex3f(x,y,0);
}
}
glEnd();

}

and assign_coordinate.c:

#include "ng.h"

GLint collisions;

GLvoid assign_coordinate(objects *tmp)
{
objects *search=o_head;

if(!tmp->prev&&object_count==1)
srand((unsigned int)time(NULL));

/* Assign coordinate planes */
if((tmp->x=malloc(sizeof(float)+7))==NULL) {
fprintf(stderr,"assign_coordinate: Could not allocate memory for x
plane\n");
perror("malloc");
exit(1);}
snprintf(o_tail->x,sizeof(float)+7,"%.0f",((grid_size-grid_start)*rand())/RAND_MAX+grid_start);
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);
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)))
{
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");
exit(1);}
if(tmp!=search&&
!strcmp(tmp->x,search->x)&&
!strcmp(tmp->y,search->y)&&
!strcmp(tmp->z,search->z)) {
collisions++;
free(tmp->x);
free(tmp->y);
free(tmp->z);
assign_coordinate(tmp);}
search=search->next;}
}

 

[Chris Dollin]

(e-mail address removed) wrote:

Side issue:

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

(a) this code appears three times. Since you give up if `malloc` fails,
abstract it into a function.

(b) as has been noted, `malloc` isn't documented as setting `errno`. What's
more, even if it did, the call of `fprintf` may itself set `errno`.
Further, if `errno` was non-zero before before `malloc` was called,
that value might persist until the `perror` call. Consequently
`perror("malloc")` is unlikely to produce a useful message.

If you /are/ going to assume that there's a useful `errno` to be
had, you're surely better off with:

errno = 0;
whatever = malloc( ... );
if (whatever == 0)
{ perror( "malloc failed for x plane" ); exit( EXIT_FAILURE ); }

 

[Chris Dollin]

(e-mail address removed) wrote:

Side issue:

PS `sizeof(float)+7`?? What on Minbar is that supposed to mean? What's
the `x` field supposed to be?

 

[Chris Dollin]

PS `sizeof(float)+7`?? What on Minbar is that supposed to mean? What's
the `x` field supposed to be?

Oh. I see.

Um. This is wrong. `sizeof(float)` has nothing to do with how many
characters are needed to represent a float as a character string.
I've just used `%.0f` format to print out a float and got:

12345678910111212

which is rather more than 11 digits (sizeof(float) being 4 on
my machine).

 

[Alef.Veld]

I know those are wrong, i will correct them. Thanks. X is a coordinate.
I was under the wrong impression that i need to give snprintf a number
of how many bytes i wanted to copy from the variable in question, be it
int,float, char, etc. At least, that's what the manpages told me. The
+7 was used to reserve some extra bytes for . precision. What would you
use then to copy a float into a char *

 

[Default User]

I know those are wrong, i will correct them. Thanks. X is a

Please don't top-post. Your replies belong following or interspersed
with properly trimmed quotes. See the majority of other posts in the
newsgroup, or:
<http://www.caliburn.nl/topposting.html>

 

[Kenny McCormack]

Please don't top-post. Your replies belong following or interspersed
with properly trimmed quotes. See the majority of other posts in the
newsgroup, or:
<http://www.caliburn.nl/topposting.html>

Get a life.

 

[Jens Thoms Toerring]

Hmm, i thought i had replied to this. Probably clicked on discard. Ok,
one more time:

In order to make clearer what's going wrong in your code I have cut it
down to a one-dimensional case, removed a few non-essential things and
removed all the error checking (which is definitely necessary but gets
a bit in the way here). I also tried to re-indent it and put in some
spaces e.g. around operators etc. for easier readability. After doing
that you end up with something like the following. I first list it as
it is and then again with my comment. Since it's not a direct copy of
your post I use a '|' instead of the of the usual '>' to indicate what
basically came from you.

| GLvoid display_grid(GLvoid)
| {
| objects *tmp = o_head;
|
| while (tmp != NULL) {
| free(tmp->x);
| assign_coordinate(tmp);
| tmp = tmp->next;
| }
| }
|
| GLvoid assign_coordinate(objects *tmp)
| {
| objects *search = o_head;
|
| tmp->x = malloc(sizeof(float) + 7));
| snprintf(o_tail->x, sizeof(float) + 7, "%.0f",
| ((grid_size - grid_start) * rand()) / RAND_MAX + grid_start);
|
| while (search != NULL) {
|
| if (collisions>0 &&
| collisions ==((grid_size - grid_start) * (grid_size -
| grid_start)))
| exit(1);
|
| if(tmp != search &&
| !strcmp(tmp->x, search->x)) {
| collisions++;
| free(tmp->x);
| assign_coordinate(tmp);
| }
|
| search=search->next;
| }
| }

I hope I didn't miss anything essential. Now again, this time with
comments.

| GLvoid display_grid(GLvoid)
| {
| objects *tmp = o_head;
|
| while (tmp != NULL) {

So here you loop over all the elements of a linked list, starting with
'o_head'. Fine so far.

| free(tmp->x);
| assign_coordinate(tmp);
| tmp = tmp->next;
| }
| }
|
|
| GLvoid assign_coordinate(objects *tmp)
| {
| objects *search = o_head;
|
| tmp->x = malloc(sizeof(float) + 7));

Here you allocate memory for the 'tmp->x' member you just before you
called assign_coordinate() had free()ed. Fine so far in that respect.
The question of your use of 'sizeof(float) + 7' I am going to address
at the end.

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

Now you print some text into 'o_tail->x' - please look again at what
you are doing here. This isn't 'tmp->x' (as you told in a different
post, 'o_tail' is the last element of the linked list and that's not
what 'tmp->x' is except possibly in a single case). It seems at least
strange that you put some random value into the last element of the
linked list and don't use the newly allocated memory, pointed to by
'tmp->x' for that purpose, especially considering what's happening
later.

| while (search != NULL) {
|
| if (collisions>0 &&
| collisions ==((grid_size - grid_start) * (grid_size -
| grid_start)))

I already told you that the comparison between an integer and a float
value for equality is rather likely not to work - you have to round
the result of the floating point calculation to the nearest integer
before you can do a meaningful comparison. Moreover, since you seem
to want to check if there are as many collisions as there are points
in your grid you make an off-by-one error. Just draw a simple grid
on paper and count the number of places where the lines intersect...

| exit(1);
|
| if(tmp != search &&
| !strcmp(tmp->x, search->x)) {

And here things go completely wrong: you never ever set the memory
'tmp->x' points to to anything, so 'tmp->x' points to uninitialized
memory. So it is definitely not suitable to be passed as an argument
to strcmp() which expects a pointer to a string. My only guess is
that you actually intend to put the random float value you produced
above into 'tmp->x' instead of 'o_tail->>x', in which case the
strcmp() here might make some sense, especially since you seem
to want to check if the random value hasn't been used somewhere
else in your list.

And now a question about something else: why do you use floating
point numbers for your grid points when you obviously only need
integers? You always round them anyway (by using the "%.0f" con-
version specifier), so why use floats at all? Lots of things are
probably going to be quite a bit simpler if you use integers in-
stead.

| collisions++;
| free(tmp->x);
| assign_coordinate(tmp);
| }
|
| search=search->next;
| }
| }

And now concerining the 'sizeof(float) + 7' thing from above (we al-
ready had an exchange about that in comp.unix.programmer and I am
going to repeat parts of what I already wrote in the hope that it
gets through this time). You asked:

I was under the wrong impression that i need to give snprintf a number
of how many bytes i wanted to copy from the variable in question, be it
int,float, char, etc. At least, that's what the manpages told me. The
+7 was used to reserve some extra bytes for . precision. What would you
use then to copy a float into a char *

1. You don't "copy" a float into a char. A float is a floating point
number in a binary representation. A floating point value in a
binary representation doesn't resemble a string like "123.46343"
im any way at all, so there's no way you could "copy" them to a
string. What you do via e.g. snprintf() is create a string repre-
sentation of that value. While a float always has the same number
of bytes (typically something in the order of 4 or 8) the string
representations of different floating point numbers can vary
widely in size - just compare e.g. "0.1" and "123422313123231.0",
both are perfectly valid floating point numbers but the first
you only need 3 characters while for second you need 17 chars
(if I counted correctly;-).

2. How many chars you need for the string representation of a float
depends on its value. If the value is e.g. 1.0e37 and you use
the "%.0f conversion specifier you will probably need 40 chars
for the string (plus one for the trailing '\0') since it will
end up as something like "10000000000000000000000000000000000000.".
You can only find out directly how many you will need by taking
the logarithm to base 10 plus 2 more (aand that for the case that
its a positive number larger than 1, there are more cases to be
taken care of). It's simpler if you have an implementation that
is C99 compliant at least for the snprintf() function, you then
can use the return value of

snprintf( NULL, 0, "%.0f", d );

(where 'd' is the value you want to represent as a string). That's
right, give it a NULL pointer as the place to print to and tell
it not to print out anything by setting the second argument to 0.
The return value then tells you how much space it would take to
print out the whole number and you then only need to add 1 for
the trailing '\0'. But if you don't have a snprintf() implemen-
tation that does the job for you you have to do the calculations
yourself, which probably will involve the use of the log10() func-
tion - or you will have to write a function that uses a first
guess for the required size, tries to print out the value in a
char array of the guessed length using snprintf(), check if that
worked out and, if it didn't, increases the value used in the
first guess, allocates more memory accordingly and then tries
again. Repeat until you had success. Those are the only options
you have as far as I can see at the moment.

Regards, Jens

PS: Please be so kind to follow the convention normally used in
technical newsgroups and don't top-post. Top-posting means
putting your text before what you are replying to. This is
rather annoying since it makes it hard to figure our what you
are reacting to. So please try to put your replies below a
citation of what you are reacting to, but make sure you only
cite what is still relevant to your reply. That way the readers
get an idea what's it all about without having to sift through
long and partly irrelevant texts. This, in turn, will increase
the likelyhood of getting useful replies to your questions.

 

[Alef.Veld]

I hope I didn't miss anything essential. Now again, this time with
comments.





Now you print some text into 'o_tail->x' - please look again at what
you are doing here. This isn't 'tmp->x' (as you told in a different
post, 'o_tail' is the last element of the linked list and that's not
what 'tmp->x' is except possibly in a single case). It seems at least
strange that you put some random value into the last element of the
linked list and don't use the newly allocated memory, pointed to by
'tmp->x' for that purpose, especially considering what's happening
later.

Aaaaargh. How stupid of me. I can't believe i did not notice that. This
explains
a lot

| while (search != NULL) {
|
| if (collisions>0 &&
| collisions ==((grid_size - grid_start) * (grid_size -
| grid_start)))

I already told you that the comparison between an integer and a float
value for equality is rather likely not to work - you have to round
the result of the floating point calculation to the nearest integer
before you can do a meaningful comparison.

I'm going to use decimals. But maybe not testing for equality, but
would it not
be possible to do 1 > 00000000.2 for example ? The reason why i
initially used
floats, is because, although i divided by ^grid in squares, i wanted .
precision
because i might want to later overlap certain squares.

And now a question about something else: why do you use floating
point numbers for your grid points when you obviously only need
integers?

See above answer.

You always round them anyway (by using the "%.0f" con-

version specifier), so why use floats at all?

I rounded them off by %.1f so i would still have 10 units of floating
point precision.

Lots of things are

probably going to be quite a bit simpler if you use integers in-
stead.

I agree. There is another reason i used floats, but i have to look that
up.

And now concerining the 'sizeof(float) + 7' thing from above (we al-
ready had an exchange about that in comp.unix.programmer and I am
going to repeat parts of what I already wrote in the hope that it
gets through this time). You asked:

number in a binary representation. A floating point value in a
binary representation doesn't resemble a string like "123.46343"
im any way at all, so there's no way you could "copy" them to a
string. What you do via e.g. snprintf() is create a string repre-
sentation of that value. While a float always has the same number
of bytes (typically something in the order of 4 or 8) the string
representations of different floating point numbers can vary
widely in size - just compare e.g. "0.1" and "123422313123231.0",
both are perfectly valid floating point numbers but the first
you only need 3 characters while for second you need 17 chars
(if I counted correctly;-).

2. How many chars you need for the string representation of a float
depends on its value. If the value is e.g. 1.0e37 and you use
the "%.0f conversion specifier you will probably need 40 chars
for the string (plus one for the trailing '\0') since it will
end up as something like "10000000000000000000000000000000000000.".
You can only find out directly how many you will need by taking
the logarithm to base 10 plus 2 more (aand that for the case that
its a positive number larger than 1, there are more cases to be
taken care of). It's simpler if you have an implementation that
is C99 compliant at least for the snprintf() function, you then
can use the return value of

snprintf( NULL, 0, "%.0f", d );

(where 'd' is the value you want to represent as a string). That's
right, give it a NULL pointer as the place to print to and tell
it not to print out anything by setting the second argument to 0.
The return value then tells you how much space it would take to
print out the whole number and you then only need to add 1 for
the trailing '\0'.

I did not know that. I have snprintf so that should work fine.

Regards, Jens

PS: Please be so kind to follow the convention normally used in
technical newsgroups and don't top-post. Top-posting means
putting your text before what you are replying to. This is
rather annoying since it makes it hard to figure our what you
are reacting to. So please try to put your replies below a
citation of what you are reacting to, but make sure you only
cite what is still relevant to your reply. That way the readers
get an idea what's it all about without having to sift through
long and partly irrelevant texts. This, in turn, will increase
the likelyhood of getting useful replies to your questions.

Personally i don't mind to read top-posts, i just scroll from up to
down instead of the other way
around. Besides, i usually want to know the answer instead of the
question so this save me
alot of time. If i think the answer is interesting, i back up. I think
the whole top-posting thing is
getting way out of hand. As long as a post is readable and not written
by a 5 year old, it should
be ok by my standards.

I also personally get quite agitated by people pushing down their
usenet etiquette down
my throat (i don't mean specifically you, but i mean those who do not
seem to have a life and
actually suspect are usenet bots . I think there is something to say
for both methods, be it top
posting or bottom posting. Having said that, i will conform with the
majority and top-post.
Halleluja

Quoting properly is a different matter. My apologies if i did not
citate properly. Since google groups
the new version went live i now use this as it is noticably faster (i
do not have to wait 5 days for a post
to be posted) but the quoting system leaves something to be desired. I
will probably revert to my usenet
agent again.

Thanks for your answers. They where really helpful.
Kind regards,
-alef

 

[Jens Thoms Toerring]

I'm going to use decimals. But maybe not testing for equality, but
would it not be possible to do 1 > 00000000.2 for example ?

Comparing for being larger or smaller is no problem but testing for
equality is. If you have e.g. something like

float a = 1.0;
a = ( a / 3.0 ) * 3.0;

if ( a == 1 )

may already fail since floating floating point calculations can only
done with a limited precision and that results in rounding errors.
And for that reason the result of first dividing 1.0 by 3.0 and after-
wards multiplying by 3.0 may be e.g. 0.99999999999 instead of 1.0, so
the test for equality with 1 (or 1.0) would fail.

I did not know that. I have snprintf so that should work fine.

Just having snprintf() is not necessarily enough - there are
some implementations (notably on Windows, at least according
to some discussion here a few days ago, but also older libc
versions) that don't work as they are supposed according to
the C99 standard. So before you decide to use that method you
better write a small test program to check if your snprintf()
does it "correctly".

Personally i don't mind to read top-posts, i just scroll from up to
down instead of the other way around. Besides, i usually want to
know the answer instead of the question so this save me alot of time.
If i think the answer is interesting, i back up. I think the whole
top-posting thing is getting way out of hand. As long as a post is
readable and not written by a 5 year old, it should be ok by my
standards.

Well, I find top-posting a PITA - I need the question to understand
what the answer is for and with top-posting I can only guess at best,
especially if this isn't just a single question with a single answer.
It's like hearing a recorded conversation but with what the parti-
cipants said all re-ordered in a more or less random way (or at least
with what was said last being replayed first). And many regulars,
especially of technical newsgroups, seem to feel the same way. Of
course, nobody can force you not to top-post, bu then you can't force
the very people that may be able to answer your question not to dis-
regard or even plonk you. Pick your choice;-)

I also personally get quite agitated by people pushing down their
usenet etiquette down my throat

If you get agitated that easily then you should consider if news-
groups are a healthy place for you to be;-) And did you consider
that usenet etiquette wasn't made up just on a whim to "push down
the throats" of unsuspecting newbies but may be the result of some
twenty years of experiences with usenet?

Regards, Jens

 

[Alef.Veld]

On Jan 27, 2:41 pm, (e-mail address removed) (Jens Thoms Toerring) wrote:

what the answer is for and with top-posting I can only guess at best,
especially if this isn't just a single question with a single answer.
It's like hearing a recorded conversation but with what the parti-
cipants said all re-ordered in a more or less random way (or at least
with what was said last being replayed first). And many regulars,
especially of technical newsgroups, seem to feel the same way. Of
course, nobody can force you not to top-post, bu then you can't force
the very people that may be able to answer your question not to dis-
regard or even plonk you. Pick your choice;-)

Well, i understand that, so like i said, i will conform to the
majority. That doesn't
mean i have to like it .

groups are a healthy place for you to be;-)

Well agitated is maybe a big word. Feeling sad for that specific
person is probably a better one. I feel there are so much
more important things then having people to top-post correctly.
Ofcourse i know now what the
next argument will be (So stop top-posting! and we can do more
important stuff!)

And did you consider

that usenet etiquette wasn't made up just on a whim to "push down
the throats" of unsuspecting newbies but may be the result of some
twenty years of experiences with usenet?

I know. I just feel that someone who thought they were important wrote
a manual like that, everyone
in return thought that it was cool to quote that person, and soon an
avalanche of top-post comments
are doing their utmost best to sanitize the usenet of those foul
topposters. I see it everywhere. Sort of
a herd mentality gone wrong ;-)

Now, I'm no newbie on usenet. Take this one user for example, he posts
with a 'Please don't top-post' and a sig referring to etiquette of
topposting, that has long been existing, yet he/she seems to pass it
off as their own work. I just think
pretty much all arguments regarding topposting are arguments i could
also make for bottomposting.

Having said that, i shall adapt to the local customs
Rgds,
Alef

 

[santosh]

On Jan 27, 2:41 pm, (e-mail address removed) (Jens Thoms Toerring) wrote:
<snip>

Now, I'm no newbie on usenet. Take this one user for example, he posts
with a 'Please don't top-post' and a sig referring to etiquette of
topposting, that has long been existing, yet he/she seems to pass it
off as their own work. I just think
pretty much all arguments regarding topposting are arguments i could
also make for bottomposting.

Having said that, i shall adapt to the local customs

One of the said local customs is not quoting the sig, unless you're
commenting on it.

 

[Richard Heathfield]

(e-mail address removed) said:

I just feel that someone who thought they were important wrote
a manual like that, everyone in return thought that it was cool
to quote that person, and soon an avalanche of top-post comments
are doing their utmost best to sanitize the usenet of those foul
topposters.

No, it's all to do with common sense and joined-up thinking, but I agree
that this isn't necessarily obvious right away. So here's the logic:

Usenet is asynchronous, and not everyone gets every article at the same time
you do. Indeed, some articles may not reach some servers at all!
Furthermore, some people don't read articles by thread, but in the order
they are received. And some newsreaders don't show articles once they've
been read.

By placing the reply *after* the text to which it refers, you provide a
little context to remind the reader of the subject under discussion (or, in
the case of someone who hasn't yet seen the article, at least give them a
fighting chance of understanding what you're talking about).

And of course it's simple common sense that the text of an answer should
come *after* the text of the question.

But I seem to hear a voice, crying in the wilderness... "I OBJECT! It takes
way too much time to scroll all the way through the previous article. No
big deal for one, but over and over again? No thanks!"

That's where the "joined-up thinking" bit comes in. The idea is to *remove*
all the text of the previous article *except* the bit(s) that you're
replying to, marking the place(s) where text has been removed with a <snip>
so that people realise that you've been editing, and that they may need to
retrieve the previous article if they want the *full* story.

If you do this properly, then your readers are not required to wade through
tons of extraneous junk to get to your reply - they have just enough
context to remind them of what you're replying to, and then there is your
reply just underneath. It works very well indeed, I assure you, and that's
why we prefer it that way.

In fact, it's top-posting that leads to excessive scrolling. First you see
the reply, then you think "huh? what's that all about?" so you have to
scroll down all the way through the reply, and then all the way through the
original article. Once you've got the context, you then have to scroll
*back to the top again*! Some of us find this so cumbersome that we simply
don't bother to read top-posted replies - it's too much like hard work.

Those who seek help on this newsgroup would do well to write their replies
in such a way that the people from whom they are seeking help do actually
bother to read those replies. Wouldn't you agree?