Re: [Pcb-src-changes] gEDA-cvs: pcb.git: branch: master updated (dbf835c02cda1d16157a83f8c1b9b1da2a05400a)

[Peter C. <pc...@ca...>]

On Sat, 2010-12-11 at 06:21 +0000, gi...@gp... wrote:
>    ConnectionTypePtr conn;
>  
> +  /* It would be worth checking if SourceNet is NULL here to avoid a segfault. Seb James. */
>    CONNECTION_LOOP (SourceNet);
>    {
>      conn = GetConnectionMemory (DestNet);

I'd rather not see comments like this added. If it is worth testing..
make a patch and add the test. Especially if the result could be a
segfault. In general, I object to sprinkling checks unnecessarily.. can
SourceNet ever be null at this point?

An assert() might make the assumption clear, a test avoids fixes the
problem (or is overly paranoid), but the comment adds nothing useful.


As a style point, if one "must" add such comments, prefix with "FIXME: "
or "XXX:", as these are picked up by many editors, and can easily be
searched for if anyone is in the mood to review such things.

Generally, I tend to prefer not to see people's names added to comments
in the code. It might put people off editing things if they think a
particular piece of code "belongs" to someone.

git log / git blame etc.. provide powerful tools to investigate the
origin of code lines - if people need to know.

-- 
Peter Clifton

Electrical Engineering Division,
Engineering Department,
University of Cambridge,
9, JJ Thomson Avenue,
Cambridge
CB3 0FA

Tel: +44 (0)7729 980173 - (No signal in the lab!)
Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)