«

pereges wrote:
> On May 9, 10:52 am, Richard Heathfield <r...@see.sig.invalid> wrote:
>
>> Debuggers are over-rated by some people, and can be a huge time sink if
>> used flailingly.
>
>
>> Yes, I agree with you that the comment about design was misguided. Module
>> boundaries should be drawn to reflect differences in functionality. The
>> idea that you have some kind of Procrustean source bed, such that any file
>> longer than N lines must be split, is silly.
>
>
>> Is that zip file ready yet?
>
> http://www.savefile.com/files/1547165
>
> Can you please provide some general tips as to how I can improve my
> style of coding ?
>
RAR files are very unfriendly for non-windows users!

A couple of points:

You don't state valid ranges for your inputs, or check them.

In find_closest_intersection, uresult and vresult and index can be used
without being assigned, which probably isn't good.

In create_reflected_rarr.efield isn't initialised.

Somewhere you are corrupting the stack.

--
Ian Collins.

On May 9, 12:59 pm, Ian Collins <ian-n...@hotmail.com> wrote:

> RAR files are very unfriendly for non-windows users!

I applogize, I don't have winZip utility on my machine.


> In find_closest_intersection, uresult and vresult and index can be used
> without being assigned, which probably isn't good.

I agree but you have to make sure that these values are accessible to
the calling function i.e. raytrace function. Hence I assigned those
values to the res variable.


> In create_reflected_rarr.efield isn't initialised.

It has been initialized in raytrace function.

On 9 May, 08:51, Richard Heathfield <r...@see.sig.invalid> wrote:
> Spiros Bousbouras said:
>
> > On 9 May, 07:20, pereges <Brol...@gmail.com> wrote:
> >>http://www.savefile.com/files/1547165
>
> > I've been to the page ; I don't see how to download
> > your file. I tried both Firefox and Lynx.
>
> I struggled too, but managed it in the end. I copied it to my site, from
> where you can easily get it:
>
> http://www.cpax.org.uk/scratch/raytrace.zip

To the OP:

In function main() you have
obj = &o;
obj = malloc(sizeof(object));
i = reader(&obj);
if(i == FAILURE)
printf("program failed in reader module\n");

The two successive assignments to obj cannot be
right. Also you don't make sure that the programme
exits in the case of FAILURE.

pereges wrote:
> On May 9, 12:59 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
>> RAR files are very unfriendly for non-windows users!
>
> I applogize, I don't have winZip utility on my machine.
>
>
>> In find_closest_intersection, uresult and vresult and index can be used
>> without being assigned, which probably isn't good.
>
> I agree but you have to make sure that these values are accessible to
> the calling function i.e. raytrace function. Hence I assigned those
> values to the res variable.
>
Yes, but they are uninitialised, so the values in res are garbage when
used. I see one of them is used as an index.

--
Ian Collins.

On May 9, 1:51 pm, Spiros Bousbouras <spi...@gmail.com> wrote:

> To the OP:
>
> In function main() you have
> obj = &o;
> obj = malloc(sizeof(object));
> i = reader(&obj);
> if(i == FAILURE)
> printf("program failed in reader module\n");
>
> The two successive assignments to obj cannot be
> right. Also you don't make sure that the programme
> exits in the case of FAILURE.

Yes I realize that mistake.

It should have been:

object o, *obj;

i = reader(&obj);

if(i == FAILURE)
{
printf("program failed in reader module!\n");
exit(1);
}

I believe there is no need for o = *obj statement either.

On May 9, 2:08 pm, Ian Collins <ian-n...@hotmail.com> wrote:

> Yes, but they are uninitialised, so the values in res are garbage when
> used. I see one of them is used as an index.

Yes, the values in res are garbage when its pointer is passed to the
find_closest_intersection function. But then, the values of the
members in res(t, u, v , tindex) are modified in that function after
intersect_triangle routine is called. The raytrace function is then
having the modified res.

......
if(intersect_triangle(r->origin, r->direction, obj->vert[obj-
>tri[i].v0], obj->vert[obj->tri[i].v1], obj->vert[obj->tri[i].v2],
&res->t, &res->u, &res->v) == TRUE)
.......

^^ The values in res are getting modified here. res->hit is modified
inside the if clause. The res->tindex is modified after the for loop
and it is assigned the value of the variable index.

On May 9, 2:16 pm, pereges <Brol...@gmail.com> wrote:

> object o, *obj;


i missed out

obj = &o;

pereges wrote:
> On May 9, 2:08 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
>> Yes, but they are uninitialised, so the values in res are garbage when
>> used. I see one of them is used as an index.
>
> Yes, the values in res are garbage when its pointer is passed to the
> find_closest_intersection function. But then, the values of the
> members in res(t, u, v , tindex) are modified in that function after
> intersect_triangle routine is called. The raytrace function is then
> having the modified res.
>
> ......
> if(intersect_triangle(r->origin, r->direction, obj->vert[obj-
>> tri[i].v0], obj->vert[obj->tri[i].v1], obj->vert[obj->tri[i].v2],
> &res->t, &res->u, &res->v) == TRUE)
> .......
>
> ^^ The values in res are getting modified here. res->hit is modified
> inside the if clause. The res->tindex is modified after the for loop
> and it is assigned the value of the variable index.

But not if the inner if is never true.

--
Ian Collins.

On 9 May 2008 at 8:22, pereges wrote:
> On May 9, 12:59 pm, Ian Collins <ian-n...@hotmail.com> wrote:
>> RAR files are very unfriendly for non-windows users!
>
> I applogize, I don't have winZip utility on my machine.

I believe Info-Zip is available as free software for Windows
(http://www.info-zip.org/).

There's a free unrarlib available on Sourceforge for *nix... presumably
someone's made a binary wrapper too if you dig around.

On May 9, 2:56 pm, Ian Collins <ian-n...@hotmail.com> wrote:

> But not if the inner if is never true.


You can check that by inserting a small printf statement. TThe inner
if is satisfied in quite a few cases.