On 8/8/11 9:34 AM, "Jeff Squyres" <jsquy...@cisco.com> wrote:

>On Aug 8, 2011, at 11:30 AM, Wesley Bland wrote:
>
>> The reason is because valgrind was complaining about uninitialized
>>values that were passed into proc_get_epoch. I saw the same warnings
>>from valgrind when I ran it. I added the code to initialize the values
>>to what really should be the default value and the warnings went away.
>>Since the process_name_t struct isn't an object, it doesn't have an
>>initialization function like so many of the other objects in the code.
>>This is what we have.
>
>Ah, I see -- you are passing peer_name into proc_get_epoch().  I missed
>that.
>
>Thanks!
>
>>     peer_name.jobid  = ORTE_PROC_MY_NAME->jobid;
>>     peer_name.vpid   = peer_idx;
>> +    peer_name.epoch  = ORTE_EPOCH_INVALID;
>>     peer_name.epoch  = orte_ess.proc_get_epoch(&peer_name);

I'm still not convinced this is a rational coding style.  It seems to me
that if you're just going to copy the epoch from peer_name to peer_name,
just assign the epoch to INVALID and be done with it.  There's no need for
both the assignment added in this patch and the line after it.

Now, normally I'd say that this is fixing a bug and having to fix other
people's bad code shouldn't be your problem.  But since you wrote both
lines, fixing it to make sense seems reasonable to me ;).

Brian

-- 
  Brian W. Barrett
  Dept. 1423: Scalable System Software
  Sandia National Laboratories






Reply via email to