Re: [DynInst_API:] Please review coverity-driven fixes

[Josh Stone <jistone@xxxxxxxxxx>]

On 12/16/2013 11:20 AM, Matthew LeGendre wrote:
> I took a walk through the ProcControlAPI/StackwalkerAPI fixes and didn't 
> see any issues.

Thanks.

> On Mon, 16 Dec 2013, Bill Williams wrote:
>> * Symtab's Object copy constructor: pretty sure it should copy all the fields 
>> if it's going to exist. The secondary point, of course, is that all 
>> Symtabs/Objects/AObjects should be reference counted and (in principle) 
>> should never be copied, so the more accurate fix is to replace them with 
>> private versions etc.

I was generally trying to stay away from semantic changes, addressing
only the problem at hand.  So if a copy constructor missed some fields,
I assumed for the moment that those fields shouldn't be copied.
Sometimes that's intentional, but still everything should get *some*
initialization.  We can easily copy more though if you want.

Since the Objects and AObjects are not in public headers, I could just
try making those into private unimplemented copy constructors, and see
if anything breaks.  I'm more wary of class Symtab though, since its
constructors are public and exported - that's a design call for you.

>> * The linux_kludges assorted fixes look much simpler and cleaner, which is 
>> great, but I also have to ask if these are still needed/used anywhere (or if 
>> it's safe to replace them with POSIX equivalents in some cases). As a general 
>> rule, things in common that are not actually domain-specific are things that 
>> we'd like to get rid of but (at the time we wrote them) there wasn't a 
>> sufficiently cross-platform alternative. (See also: pdvector. The cruft goes 
>> a long way back here.)

These rewritten functions are a bit of an exception as this patch series
goes, but I still wasn't looking much further than the functions
themselves.  Maybe it deserves a wider audit in the near future.

In general, I agree, some things could use more from POSIX, or failing
that, from boost.  After I finish my compiler-warnings and
static-analyzer crusade, maybe I can try my hand at some of these more
general janitorial tasks.  Got a list?

>> * Not 100% positive that all of the fallthroughs that you left as fallthrough 
>> are non-bugs, but in the worst case they're not making behavior any worse. 
>> I'll review those in more detail this afternoon and see if I find anything 
>> that needs fixing.

Sure, let me know.  This is why I separated my commits for intentional
fallthrough vs added breaks, so this would stand out.


Thanks,
Josh