On May 10, 11:31 am, Richard Heathfield <r...@[EMAIL PROTECTED]
> wrote:
> I promised a reply to this, and here it is.
Thank you very much for the help. It has given me a lot of insight on
C programming in general.
> Okay, so let's look at the code. You should of course read other
people's
> replies too, because I'm bound to miss something.
>
> I indented the code to my own taste. It had tabs in, and they had to go,
> because Usenet hates tabs. Line numbers refer to /my/ version of the
code,
> which was the result of indenting the original with these settings:
I understand. What text editors do you personally prefer while working
on linux ? What about vi ?On windows machine, I use Qeditor
> The first problem I found was that the project wouldn't compile. That
> probably means that you are using C99isms - not wrong, but inconvenient
> for those few remaining millions that don't have a C99 compiler. To get
> your code to compile, I must either disable C90 conformance in my
compiler
> or hack the code. Since I don't plan on disabling conformance,
> code-hackery happened at this end.
>
> Rule 1 is of course to fix the first diagnostic message first. Now, in
this
> case, the first few messages were:
>
> raytrace.c: In function `find_closest_intersection':
> raytrace.c:29: warning: `index' might be used uninitialized in this
> function
> raytrace.c: In function `raytrace':
> raytrace.c:67: warning: unused variable `i'
>
> The first of these looks dire, but it may well be that obj->ntri is
always
> greater than 0.
Yes obj->ntri >= 0
At this stage, however, I don't know that. And it would
> not hurt to initialise index to -1 at this stage. If it remains -1 after
> the loop, the original program was clearly assuming that the loop was
> always setting this value, and therefore was broken, so I'm going to
> assert that it's not -1... in fact, I'm going to assert that it's
> non-negative (because it's set to i, which runs from 0 upwards). This
> involves adding <assert.h> to the include list, and changing
>
> int index, i;
>
> to
>
> int index = -1, i;
>
> and adding
>
> assert(index >= 0);
>
> just after the for-loop and just before res->t = closestdistance;
I think this is not the problem. It is possible that index will be -1
for many rays and its perfectly normal. The index value is just there
to indicate which triangle the ray has hit. It is possible that the
ray does not hit any triangle. I have a member in the intersectresult
data structure called 'hit'. If the ray does not hit any triangle,
then hit will remain FALSE. index will probably contain some garbage
value. But that's irrelevant since in the raytrace function we are
looking for only those cases where res.hit == TRUE.
> Second fix to this file: in the raytrace() function, i is unused, so I
> removed it.
Do you think its dangerous to have such declarations in bigger
projects ?
> Well, I had to dig through three layers of #include before finding an
> include for math.h. Not funny. Still, it's there somewhere.
My idea was to have such declarations at one place. Hence, I included
it in common.h.
>
> The other stuff? Well, not your fault - you're using valid C99 syntax -
but
> my compiler doesn't sup****t it so I'm going to rewrite it into a syntax
> that is sup****ted by your compiler and mine. I will take the op****tunity
> to point out some assumptions that your code is making by asserting that
> they are true. I've also re-written your malloc in canonical style:
>
> *pointinarray = malloc(*numpoints * sizeof **pointinarray);
>
> In general, for any object pointer p, you can point it at the memory for
N
> objects of that type by calling malloc like this:
>
> p = malloc(N * sizeof *p);
>
> Note the leading * on the rightmost p.
>
> Plugging in: for N, you want *numpoints, right? So:
>
> p = malloc(*numpoints * sizeof *p);
>
> For p, you want *pointinarray. Watch carefully, counting stars:
>
> *pointinarray = malloc(*numpoints * sizeof **pointinarray);
>
> And that's it. This form survives a type change without modification
> (except, of course, a type change to void *).
>
> I am very suspicious about these lines:
>
> for(yi = 0; yi <= numpointsy; ++yi)
> {
> for(xi = 0; xi <= numpointsx; ++xi)
>
> How SURE are you that those <= are correct? It's very likely that they
> should be < rather than <= but you as the code's author will be able to
> work out whether this is an error far more quickly than I can.
>
> My revision of your gen_grid_points function (which requires you to add
an
> include for the <assert.h> header) looks like this:
>
> int gen_grid_points(vector ** pointinarray,
> unsigned long int *numpoints,
> double xmax,
> double xmin,
> double ymax,
> double ymin,
> double zdist)
> {
> unsigned long int numpointsx;
> unsigned long int numpointsy;
> unsigned long int i = 0;
> double xlength = xmax - xmin;
> double ylength = ymax - ymin;
> double xsize;
> double ysize;
> unsigned long int xi, yi;
>
> assert(pointinarray != NULL);
>
> assert(numpoints != NULL);
> assert(*numpoints >= 0);
>
> assert(xmax >= xmin);
> assert(ymax >= ymin);
>
> numpointsx = sqrt(*numpoints);
> numpointsy = sqrt(*numpoints);
>
> assert(numpointsx > 1);
> assert(numpointsy > 1);
>
> xsize = xlength / (numpointsx - 1);
> ysize = ylength / (numpointsy - 1);
>
> *numpoints = (numpointsx + 1) * (numpointsy + 1);
>
> *pointinarray = malloc(*numpoints * sizeof **pointinarray);
>
> if(*pointinarray == NULL)
> {
> perror("malloc has failed");
> return FAILURE;
> }
>
> /* Generating the points on the grid, points to be used as ray origin
*/
> for(yi = 0; yi <= numpointsy; ++yi)
> {
> for(xi = 0; xi <= numpointsx; ++xi)
> {
> (*pointinarray + i)->x = xmin + xi * xsize;
> (*pointinarray + i)->y = ymin + yi * ysize;
> (*pointinarray + i)->z = zdist;
> ++i;
> }
> }
>
> return SUCCESS;
>
> }
>
> I recommend that you plug this version in, and re-test. Do any of the
> assertions fail? If so, that may well be your problem.
Yes, there is some problem while compiling radar.c.
The problem is with the assert(*numpoints >= 0) statement on line
number 32 of radar.c as re****ted by pellesC compiler:
Building radar.obj.
C:\Do***ents and Settings\abc\Desktop\raytrace\raytrace\radar.c(32):
warning #2130: Result of unsigned comparison is constant.
C:\Do***ents and Settings\abc\Desktop\raytrace\raytrace\radar.c(32):
fatal error: Internal error: 'Access violation' at 0x004083f3.
I tried to comment that line and then compiled and executed the code.
I get the same output as before for the following input:
frequency: 40e+7
amplitude: 10e-4
numberofrays: 1000
The output is:
Es: 7.085529e-11 Ei: 7.957116e-05 RCS: 1.118991e+01
BartC has obtained similar results using the PellesC compiler but he
is getting different result when using dmc, lccwin, gcc on
windows( all the three results using dmc, lccwin, gcc are same though
for the above input). Can you please tell me what result you have
obtained on your linux machine ? Are the results consistent everytime
the program is executed ?
|
