Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.arch.embedded > #18194 > unrolled thread

D'oh! Oh, for lack of a code review.

Back to article view | Back to comp.arch.embedded

Contents

  D'oh!  Oh, for lack of a code review. Tim Wescott <seemywebsite@myfooter.really> - 2014-09-21 16:23 -0500
    Re: D'oh!  Oh, for lack of a code review. Clifford Heath <no.spam@please.net> - 2014-09-22 09:02 +1000
      Re: D'oh!  Oh, for lack of a code review. Simon Clubley <clubley@remove_me.eisner.decus.org-Earth.UFP> - 2014-09-22 00:21 +0000
        Re: D'oh!  Oh, for lack of a code review. Clifford Heath <no.spam@please.net> - 2014-09-22 10:58 +1000
          Re: D'oh! Oh, for lack of a code review. Les Cargill <lcargill99@comcast.com> - 2014-09-21 22:28 -0500
        Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-21 22:16 -0400
      Re: D'oh!  Oh, for lack of a code review. Paul E Bennett <Paul_E.Bennett@topmail.co.uk> - 2014-09-22 01:28 +0100
        Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-21 20:16 -0700
          Re: D'oh!  Oh, for lack of a code review. Bruce Varley - 2014-09-22 11:51 +0800
            Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-21 21:19 -0700
          Re: D'oh!  Oh, for lack of a code review. Paul E Bennett <Paul_E.Bennett@topmail.co.uk> - 2014-09-22 22:20 +0100
            Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:28 -0700
              Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 00:43 -0400
          Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-22 18:48 -0400
            Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 15:59 -0700
              Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 00:49 -0400
      Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:15 -0700
        Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:30 -0700
    Re: D'oh!  Oh, for lack of a code review. David Brown <david.brown@hesbynett.no> - 2014-09-22 10:22 +0200
    Re: D'oh!  Oh, for lack of a code review. Hans-Bernhard Bröker <HBBroeker@t-online.de> - 2014-09-22 19:08 +0200
      Re: D'oh!  Oh, for lack of a code review. Tim Wescott <seemywebsite@myfooter.really> - 2014-09-22 13:59 -0500
        Re: D'oh!  Oh, for lack of a code review. Paul Rubin <no.email@nospam.invalid> - 2014-09-22 12:18 -0700
          Re: D'oh!  Oh, for lack of a code review. tim <tim@seemywebsite.com> - 2014-09-23 00:27 -0500
            Re: D'oh!  Oh, for lack of a code review. Robert Wessel <robertwessel2@yahoo.com> - 2014-09-23 18:35 -0500
        Re: D'oh!  Oh, for lack of a code review. David Brown <david.brown@hesbynett.no> - 2014-09-22 21:26 +0200
    Re: D'oh!  Oh, for lack of a code review. John Speth <johnspeth@yahoo.com> - 2014-09-22 16:24 -0700
      Re: D'oh!  Oh, for lack of a code review. Don Y <this@is.not.me.com> - 2014-09-22 16:34 -0700
      Re: D'oh!  Oh, for lack of a code review. rickman <gnuarm@gmail.com> - 2014-09-23 01:11 -0400

Page 1 of 2  [1] 2  Next page →

#18194 — D'oh! Oh, for lack of a code review.

I just had to share this, god knows why.  I keep thinking that I want to 
figure out a way to gather up a group of five or ten embedded software 
consultants in my area and form a code review exchange, to keep things 
like this from happening.  See if YOU can find the error!

I'm not sure why the Gnu compiler bitches if I compare an int to an 
unsigned int, but doesn't bitch when I compare a boolean to an int -- but 
there you are.

class CPort
{
...
  bool elapsed()          {return _elapsed;}
...
}

...

CPort _port;

...

if (_port.elapsed() > 0 && 
	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
{
 ...
}

-- 

Tim Wescott
Wescott Design Services
http://www.wescottdesign.com

[toc] | [next] | [standalone]

#18198

On 22/09/14 07:23, Tim Wescott wrote:
> See if YOU can find the error!
> if (_port.elapsed() > 0 &&
> 	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))

0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
Do I get a cookie?

This is why we should be using a language with a proper type system.

[toc] | [prev] | [next] | [standalone]

#18203

On 2014-09-21, Clifford Heath <no.spam@please.net> wrote:
> On 22/09/14 07:23, Tim Wescott wrote:
>> See if YOU can find the error!
>> if (_port.elapsed() > 0 &&
>> 	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>
> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
> Do I get a cookie?
>
> This is why we should be using a language with a proper type system.

I've just encountered the exact same thing this evening, with unsigned
long int being silently truncated to unsigned char without a warning.

I needed to extend a return value from unsigned char to unsigned long
int after a spec revealed an additional usage case. I knew there was
additional code which needed changing but I did a quick compile just
to check for syntax errors in the changes to that point and the damn
thing compiled without a single warning. :-(

And I agree, you wouldn't get that past an Ada compiler.

Simon.

PS: And yes, -Wall -Werror was enabled.

-- 
Simon Clubley, clubley@remove_me.eisner.decus.org-Earth.UFP
Microsoft: Bringing you 1980s technology to a 21st century world

[toc] | [prev] | [next] | [standalone]

#18206

On 22/09/14 10:21, Simon Clubley wrote:
> On 2014-09-21, Clifford Heath <no.spam@please.net> wrote:
>> On 22/09/14 07:23, Tim Wescott wrote:
>>> See if YOU can find the error!
>>> if (_port.elapsed() > 0 &&
>>> 	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>>
>> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
>> This is why we should be using a language with a proper type system.
> And I agree, you wouldn't get that past an Ada compiler.

Ada's type system is strong, but terribly intrusive, worse even than 
Java (which isn't even strongly-typed). It's that kind of intrusiveness 
that drives folk like Paul away from typed languages - though those have 
their place; I like Ruby, with high-coverage BDD.

Modern languages like Haskell implement strong typing using type 
inferencing (you almost never need type annotations), which make them 
just as nice and succinct to write in as untyped scripting languages, 
but as safe or safer than Ada. They're becoming viable for embedded 
work, too, though I suspect more evolution is needed. Haskell is lazy, 
for example, which makes it hard to reason about performance. Rust is a 
newer contender though.

Type inferencing is the way of the future, in any case.

Clifford Heath.

[toc] | [prev] | [next] | [standalone]

#18217 — Re: D'oh! Oh, for lack of a code review.

Clifford Heath wrote:
> On 22/09/14 10:21, Simon Clubley wrote:
>> On 2014-09-21, Clifford Heath <no.spam@please.net> wrote:
>>> On 22/09/14 07:23, Tim Wescott wrote:
>>>> See if YOU can find the error!
>>>> if (_port.elapsed() > 0 &&
>>>>     _port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>>>
>>> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
>>> This is why we should be using a language with a proper type system.

All languages have a "proper type system." You - yes, you , you're the
sentient creature in this relationship, presumably - just have to know 
what you're doing.

The types in 'C' just happen to have a referent to the physical types of
the hardware, usually. This is okay.

The time to complain about 'C', like the time to plant a tree, is thirty 
years ago.

Go to about 58:00 or so in this video
http://www.infoq.com/presentations/Null-References-The-Billion-Dollar-Mistake-Tony-Hoare

Side effects; they're what's for dinner.

>> And I agree, you wouldn't get that past an Ada compiler.
>
> Ada's type system is strong, but terribly intrusive, worse even than
> Java (which isn't even strongly-typed). It's that kind of intrusiveness
> that drives folk like Paul away from typed languages - though those have
> their place; I like Ruby, with high-coverage BDD.
>
> Modern languages like Haskell implement strong typing using type
> inferencing (you almost never need type annotations), which make them
> just as nice and succinct to write in as untyped scripting languages,
> but as safe or safer than Ada. They're becoming viable for embedded
> work, too, though I suspect more evolution is needed. Haskell is lazy,
> for example, which makes it hard to reason about performance. Rust is a
> newer contender though.
>
> Type inferencing is the way of the future, in any case.
>
> Clifford Heath.


It's always thirty years in the future, and has been for at least
thirty years. I can name a dozen solutions - one's still alive; Bruce 
Baden Powell - oops, Bruce Powell Douglass' use of executable UML.
Its darned close to provably correct if you're strenuous enough
about it.

Nobody uses it.

-- 
Les Cargill

[toc] | [prev] | [next] | [standalone]

#18213

On 9/21/2014 8:21 PM, Simon Clubley wrote:
> On 2014-09-21, Clifford Heath <no.spam@please.net> wrote:
>> On 22/09/14 07:23, Tim Wescott wrote:
>>> See if YOU can find the error!
>>> if (_port.elapsed() > 0 &&
>>> 	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>>
>> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
>> Do I get a cookie?
>>
>> This is why we should be using a language with a proper type system.
>
> I've just encountered the exact same thing this evening, with unsigned
> long int being silently truncated to unsigned char without a warning.

I don't use C++ but when I did C programming I would clean my code to 
get rid of all warnings while the other coders said I was being silly as 
the warnings were just "warnings".  lol

I thought this type of thing *would* produce a warning.  Are the 
compiler flags set to not give this sort of warning?


> I needed to extend a return value from unsigned char to unsigned long
> int after a spec revealed an additional usage case. I knew there was
> additional code which needed changing but I did a quick compile just
> to check for syntax errors in the changes to that point and the damn
> thing compiled without a single warning. :-(
>
> And I agree, you wouldn't get that past an Ada compiler.
>
> Simon.
>
> PS: And yes, -Wall -Werror was enabled.

Oh, ok.

Everyone complains about the strong typing in VHDL.  It's the problems 
you don't see because you couldn't make them that it saves you from. 
Hard to show people those things.

-- 

Rick

[toc] | [prev] | [next] | [standalone]

#18204

Clifford Heath wrote:

> On 22/09/14 07:23, Tim Wescott wrote:
>> See if YOU can find the error!
>> if (_port.elapsed() > 0 &&
>> _port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
> 
> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
> Do I get a cookie?
> 
> This is why we should be using a language with a proper type system.

I use a language that has no sense of typing and can manage to produce 
correct code without too much fuss.

I like Tim's idea of forming Code Review Circles, which is one way of 
keeping the "Lone Wolf Developer" from going too insane. Fortunately I have 
a few friends that will help review material for me (documents as well as 
code and drawings). I do similar for them too. It is part of what networking 
is all about.

-- 
********************************************************************
Paul E. Bennett IEng MIET.....<email://Paul_E.Bennett@topmail.co.uk>
Forth based HIDECS Consultancy.............<http://www.hidecs.co.uk>
Mob: +44 (0)7811-639972
Tel: +44 (0)1235-510979
Going Forth Safely ..... EBA. www.electric-boat-association.org.uk..
********************************************************************

[toc] | [prev] | [next] | [standalone]

#18215

On 9/21/2014 5:28 PM, Paul E Bennett wrote:
> I like Tim's idea of forming Code Review Circles, which is one way of
> keeping the "Lone Wolf Developer" from going too insane. Fortunately I have
> a few friends that will help review material for me (documents as well as
> code and drawings). I do similar for them too. It is part of what networking
> is all about.

I think this is only practical in "dense" areas -- and, only with contractors.
Folks who have 9-to-5's tend to want to leave work behind at 5 (and would
probably not be compensated to do this sort of thing on the company's dime!)

You can do this "long distance" but in my experiences, it loses a lot of
the intimacy and energy that "in person" reviews seem to exhibit.  And,
a lot more gets overlooked (because of the "cost" of bringing someone's
attention to an issue -- wondering if everyone else has already made a
similar observation).

It's *really* nice to have a bunch of heads looking at the *same* thing...
knowing where each other's gaze/focus is drifting, etc.

It's a LOT easier to do hardware reviews remotely.  Especially with
interactive video, etc.  "Everything" is visible at one time -- to
everyone, etc.

[toc] | [prev] | [next] | [standalone]

#18218

On Sun, 21 Sep 2014 20:16:47 -0700, Don Y <this@is.not.me.com> wrote:

>On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>> I like Tim's idea of forming Code Review Circles, which is one way of
>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I have
>> a few friends that will help review material for me (documents as well as
>> code and drawings). I do similar for them too. It is part of what networking
>> is all about.
>
>I think this is only practical in "dense" areas -- and, only with contractors.
>Folks who have 9-to-5's tend to want to leave work behind at 5 (and would
>probably not be compensated to do this sort of thing on the company's dime!)
>
>You can do this "long distance" but in my experiences, it loses a lot of
>the intimacy and energy that "in person" reviews seem to exhibit.  And,
>a lot more gets overlooked (because of the "cost" of bringing someone's
>attention to an issue -- wondering if everyone else has already made a
>similar observation).
>
>It's *really* nice to have a bunch of heads looking at the *same* thing...
>knowing where each other's gaze/focus is drifting, etc.
>
>It's a LOT easier to do hardware reviews remotely.  Especially with
>interactive video, etc.  "Everything" is visible at one time -- to
>everyone, etc.

Good topic for airing from a number of angles. I'm part of a very
small team developing specialised code for large manufacturing plants,
using arcane languages and platforms that mean that an 'ordinary'
programmer isn't going to be of much help.  I've made the point to our
team leaders that in the wider embedded software industry, code
overview/reviewing is a strongly recommended practice, but for the
usual reasons, getting anyone assigned to the job never happens.

The few times I've managed to get a colleague, unofficially, to do a
review for me, the results haven't been great.  It's relatively easy
to catch the superficial stuff, such as pointless looping and crappy
naming. But the things that cause trouble are embedded much deeper,
and catching them would require a thorough understanding of the
situation - a programmer-to-programmer brain transplant almost - to
achieve much.  I'd be interested in how this is dealt with by others.
How counterproductive is lack of extreme detail in the functional
specification? Is there such a thing as a methodology that people
would recommend?  Is top-down or bottom up better (and is this
question meaningful?). Is it possible to estimate review time per
line, or on some other basis?  

[toc] | [prev] | [next] | [standalone]

#18220

On 9/21/2014 8:51 PM, "Bruce Varley" wrote:

> Good topic for airing from a number of angles. I'm part of a very
> small team developing specialised code for large manufacturing plants,
> using arcane languages and platforms that mean that an 'ordinary'
> programmer isn't going to be of much help.

You'd be surprised at how many parallels pertain to other industries,
regardless of languages, platforms, etc.

I.e., application domains tend to be fairly specialized (unless you're
writing desktop code -- though even there!).  A person developing
motor control code and another developing software to analyze blood
samples have very little common ground (besides, perhaps, a language).

> I've made the point to our
> team leaders that in the wider embedded software industry, code
> overview/reviewing is a strongly recommended practice, but for the
> usual reasons, getting anyone assigned to the job never happens.

IME, you can't treat it as a checkoff item that happens at a
specific place(s) in the development process.  The results are
much better if your colleagues are intimately aware of the
project, have discussed aspects of it with you in the past, etc.

At the very least, they feel less like "outsiders" and more
"invested" in the (your) project.

One place I worked had token rewards (trivial things -- a candy bar!)
to acknowledge value contributed by others to "your" project.  Sounds
silly but it makes the event more memorable to all parties.

> The few times I've managed to get a colleague, unofficially, to do a
> review for me, the results haven't been great.  It's relatively easy
> to catch the superficial stuff, such as pointless looping and crappy
> naming. But the things that cause trouble are embedded much deeper,
> and catching them would require a thorough understanding of the
> situation - a programmer-to-programmer brain transplant almost - to
> achieve much.

Exactly.  Doing this after the fact is like proofreading a novel:
yeah, you might catch some spelling, punctuation, grammatical
errors -- but, any comments you have on the storyline are too late
to be effective (then why bother offering them?).

OTOH, if you'd been discussing your "plot line" with that future
proofreader all along, you might have been able to benefit from
these observations early on.

> I'd be interested in how this is dealt with by others.
> How counterproductive is lack of extreme detail in the functional
> specification?

I find a detailed specification makes things a LOT easier to
critique.  I now write user manuals early on as these let
folks (clients, potential users, myself, etc.) envision REAL
situations and how they will be handled.  It's amazing
how many big holes you can find in a client's proposal once
you start rewriting it in this form ("gee, we have a way of
CREATING foobars but no way of DELETING them!  Oh, and, by
the way, how many of these can we create?  And, how does this
affect the usability of the system?")

> Is there such a thing as a methodology that people
> would recommend?

I spend a lot of time documenting exceptions and special cases
in my source commentary.  "I'm doing it this way BECAUSE..."

In recent projects, I am removing this from the codebase and,
instead, preparing "tutorials" -- essentially, organizing my
working notes in a manner that allows others to see how I
came to a particular decision (instead of just documenting
the decision itself).

This also helps identify the cases that *need* to be verified
when the code is tested.  And, lets others examine the test
suite to consider other situations that don't appear, there.

> Is top-down or bottom up better (and is this
> question meaningful?). Is it possible to estimate review time per
> line, or on some other basis?

I've never seen any productive results doing "line by line" style
reviews.  People QUICKLY "zone-out".  It's too tedious and yields
too few "hits".  Rather, indicate WHAT you are trying to do and HOW
you are doing it.  Let your reviewers ASSUME you have implemented
this correctly (why examine every line of code?).  Indicate how
you have tested it (did you forget some boundary condition?).
Then, let them think about what COULD go wrong with your overall
approach -- and YOU explain why that's NOT a problem...

"What happens if we want to create 5,000 foobars?  How would a user
manage all of those 'objects'/names?"

[toc] | [prev] | [next] | [standalone]

#18269

Don Y wrote:

> On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>> I like Tim's idea of forming Code Review Circles, which is one way of
>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I
>> have a few friends that will help review material for me (documents as
>> well as code and drawings). I do similar for them too. It is part of what
>> networking is all about.
> 
> I think this is only practical in "dense" areas -- and, only with
> contractors. Folks who have 9-to-5's tend to want to leave work behind at
> 5 (and would probably not be compensated to do this sort of thing on the
> company's dime!)
> 
> You can do this "long distance" but in my experiences, it loses a lot of
> the intimacy and energy that "in person" reviews seem to exhibit.  And,
> a lot more gets overlooked (because of the "cost" of bringing someone's
> attention to an issue -- wondering if everyone else has already made a
> similar observation).
> 
> It's *really* nice to have a bunch of heads looking at the *same* thing...
> knowing where each other's gaze/focus is drifting, etc.
> 
> It's a LOT easier to do hardware reviews remotely.  Especially with
> interactive video, etc.  "Everything" is visible at one time -- to
> everyone, etc.

I would never suggest having a whole software project dumped on the reviewer 
in one go. However, if you follow the rules about creating readable code, 
decompose functionality and partition well enough you should be able to get 
small bits reviewed no matter how remote your review partner is. 

They need sight of the (part) specification that led to your design/code so 
that they can draw their own conclusions about whether you were on the right 
track or were making things over complex.

-- 
********************************************************************
Paul E. Bennett IEng MIET.....<email://Paul_E.Bennett@topmail.co.uk>
Forth based HIDECS Consultancy.............<http://www.hidecs.co.uk>
Mob: +44 (0)7811-639972
Tel: +44 (0)1235-510979
Going Forth Safely ..... EBA. www.electric-boat-association.org.uk..
********************************************************************

[toc] | [prev] | [next] | [standalone]

#18282

Hi Paul,

On 9/22/2014 2:20 PM, Paul E Bennett wrote:
> Don Y wrote:
>
>> On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>>> I like Tim's idea of forming Code Review Circles, which is one way of
>>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I
>>> have a few friends that will help review material for me (documents as
>>> well as code and drawings). I do similar for them too. It is part of what
>>> networking is all about.
>>
>> I think this is only practical in "dense" areas -- and, only with
>> contractors. Folks who have 9-to-5's tend to want to leave work behind at
>> 5 (and would probably not be compensated to do this sort of thing on the
>> company's dime!)
>>
>> You can do this "long distance" but in my experiences, it loses a lot of
>> the intimacy and energy that "in person" reviews seem to exhibit.  And,
>> a lot more gets overlooked (because of the "cost" of bringing someone's
>> attention to an issue -- wondering if everyone else has already made a
>> similar observation).
>>
>> It's *really* nice to have a bunch of heads looking at the *same* thing...
>> knowing where each other's gaze/focus is drifting, etc.
>>
>> It's a LOT easier to do hardware reviews remotely.  Especially with
>> interactive video, etc.  "Everything" is visible at one time -- to
>> everyone, etc.
>
> I would never suggest having a whole software project dumped on the reviewer
> in one go. However, if you follow the rules about creating readable code,
> decompose functionality and partition well enough you should be able to get
> small bits reviewed no matter how remote your review partner is.
>
> They need sight of the (part) specification that led to your design/code so
> that they can draw their own conclusions about whether you were on the right
> track or were making things over complex.

Yes, but when you start slicing a project up into "manageable chunks"
to examine individually, it takes a fair bit of *concerted* effort to
remember what the roles of each of the modules you've already reviewed
were -- especially any assumptions that you made IN those reviews

IME, it's the assumptions in which bugs lie -- all the unspoken things
that you are likely to assume ONE WAY when looking at THIS; then equally
likely to assume some OTHER way when looking at THAT!  If you forget
how your internal bias resolved those assumptions at any point, you
leave yourself open to a bug slipping in under your nose.

I can't *count* the number of times I have crashed someone's code
and been met with:
    "What did you do?"
    <yada_yada_yada>
    "But, you're not SUPPOSED to do that!  That's stupid!!"
    "Then, why did you LET ME?"
    <mumble>  "Well, I assumed..."
    "What?  You assumed the bank robber wouldn't walk into an unlocked vault?"

First thing to do in any project (hardware or software) review is to
identify all the unspoken assumptions that the developer(s) made.
And, then, VIOLATE those as you examine the implementation.

E.g., I will deliberately choose a do-loop over a while-loop when I
am *sure* there will be at least one iteration (assuming "for" is
not appropriate).  This is my way of signaling that this code
*will* be executed -- at least once!  I then preface the code with
commentary and suitable invariants that explains why this is so.

As a result, those instances where I allow ZERO iterations stand
out and let others ask "what happens if this whole block gets
skipped over?  what might break??" (as that is a likely source
of errors)

[the "do-loops" are then implicitly known to NOT be skipped!]

Familiarity with each other's design/coding styles can make these
sorts of things a lot easier to sort out.  When I design hardware,
I am a big fan of FSM's.  So, folks already know what to expect
when they examine one of my hardware implementations:  clock is
king!  make sure to explore skews between clocks!!

[toc] | [prev] | [next] | [standalone]

#18296

On 9/22/2014 7:28 PM, Don Y wrote:
>
> Yes, but when you start slicing a project up into "manageable chunks"
> to examine individually, it takes a fair bit of *concerted* effort to
> remember what the roles of each of the modules you've already reviewed
> were -- especially any assumptions that you made IN those reviews

There is a thing called requirements allocation that deals with imposing 
specific requirements on specific modules.  Then new requirements that 
only apply to lower level entities are created *and* documented so the 
person designing the lower modules knows how to make their code play 
well with the other modules.

This is not so hard, but it has to be rigorous.


> IME, it's the assumptions in which bugs lie -- all the unspoken things
> that you are likely to assume ONE WAY when looking at THIS; then equally
> likely to assume some OTHER way when looking at THAT!  If you forget
> how your internal bias resolved those assumptions at any point, you
> leave yourself open to a bug slipping in under your nose.

There should be *no* assumptions.  Assumptions are written down and that 
means they are part of the spec and not assumptions.

...snip multiple paragraphs that say what I just wrote above...

> Familiarity with each other's design/coding styles can make these
> sorts of things a lot easier to sort out.  When I design hardware,
> I am a big fan of FSM's.  So, folks already know what to expect
> when they examine one of my hardware implementations:  clock is
> king!  make sure to explore skews between clocks!!

What are you designing that the clock has skew enough to matter?  Clock 
skew is dealt with as that, clock skew.  It is minimized enough that it 
never affects a design.  Why would you need to consider it any further? 
  The impact of too much clock skew is always a problem.

-- 

Rick

[toc] | [prev] | [next] | [standalone]

#18275

On 9/21/2014 11:16 PM, Don Y wrote:
> On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>> I like Tim's idea of forming Code Review Circles, which is one way of
>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I
>> have
>> a few friends that will help review material for me (documents as well as
>> code and drawings). I do similar for them too. It is part of what
>> networking
>> is all about.
>
> I think this is only practical in "dense" areas -- and, only with
> contractors.
> Folks who have 9-to-5's tend to want to leave work behind at 5 (and would
> probably not be compensated to do this sort of thing on the company's
> dime!)
>
> You can do this "long distance" but in my experiences, it loses a lot of
> the intimacy and energy that "in person" reviews seem to exhibit.  And,
> a lot more gets overlooked (because of the "cost" of bringing someone's
> attention to an issue -- wondering if everyone else has already made a
> similar observation).
>
> It's *really* nice to have a bunch of heads looking at the *same* thing...
> knowing where each other's gaze/focus is drifting, etc.
>
> It's a LOT easier to do hardware reviews remotely.  Especially with
> interactive video, etc.  "Everything" is visible at one time -- to
> everyone, etc.

These days hardware *is* software.  The last company I worked for had 
the FPGA people in the software department.

-- 

Rick

[toc] | [prev] | [next] | [standalone]

#18278

On 9/22/2014 3:48 PM, rickman wrote:
> On 9/21/2014 11:16 PM, Don Y wrote:
>> On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>>> I like Tim's idea of forming Code Review Circles, which is one way of
>>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I
>>> have
>>> a few friends that will help review material for me (documents as well as
>>> code and drawings). I do similar for them too. It is part of what
>>> networking
>>> is all about.
>>
>> I think this is only practical in "dense" areas -- and, only with
>> contractors.
>> Folks who have 9-to-5's tend to want to leave work behind at 5 (and would
>> probably not be compensated to do this sort of thing on the company's
>> dime!)
>>
>> You can do this "long distance" but in my experiences, it loses a lot of
>> the intimacy and energy that "in person" reviews seem to exhibit.  And,
>> a lot more gets overlooked (because of the "cost" of bringing someone's
>> attention to an issue -- wondering if everyone else has already made a
>> similar observation).
>>
>> It's *really* nice to have a bunch of heads looking at the *same* thing...
>> knowing where each other's gaze/focus is drifting, etc.
>>
>> It's a LOT easier to do hardware reviews remotely.  Especially with
>> interactive video, etc.  "Everything" is visible at one time -- to
>> everyone, etc.
>
> These days hardware *is* software.  The last company I worked for had the FPGA
> people in the software department.

You can far more easily represent a piece of hardware on a "unified drawing"
than you can with software.  So, you can provide a "block/functional diagram"
that allows everyone to see the overall structure of a circuit; then, delve
into selected "subsections" as appropriate.

Tools (and "thought processes") for doing this with a piece of software
just don't exist in widespread use.

This is why software is considerably harder to document, describe, analyze
(in an isolated human brain), etc.

*When* adequate tools (and methodologies) exist to provide (and are adopted)
these same sorts of "illustrations" that allow entire software systems to be
"conceptualized" in such a terse, abstract manner, I suspect there will be
a quantum leap in software reliability, etc.

     "Why is this module talking to this *other* module?  Is that intentional?
     It *smells* like a potential problem area... let's take a look inside
     those two modules!"

[toc] | [prev] | [next] | [standalone]

#18297

On 9/22/2014 6:59 PM, Don Y wrote:
> On 9/22/2014 3:48 PM, rickman wrote:
>> On 9/21/2014 11:16 PM, Don Y wrote:
>>> On 9/21/2014 5:28 PM, Paul E Bennett wrote:
>>>> I like Tim's idea of forming Code Review Circles, which is one way of
>>>> keeping the "Lone Wolf Developer" from going too insane. Fortunately I
>>>> have
>>>> a few friends that will help review material for me (documents as
>>>> well as
>>>> code and drawings). I do similar for them too. It is part of what
>>>> networking
>>>> is all about.
>>>
>>> I think this is only practical in "dense" areas -- and, only with
>>> contractors.
>>> Folks who have 9-to-5's tend to want to leave work behind at 5 (and
>>> would
>>> probably not be compensated to do this sort of thing on the company's
>>> dime!)
>>>
>>> You can do this "long distance" but in my experiences, it loses a lot of
>>> the intimacy and energy that "in person" reviews seem to exhibit.  And,
>>> a lot more gets overlooked (because of the "cost" of bringing someone's
>>> attention to an issue -- wondering if everyone else has already made a
>>> similar observation).
>>>
>>> It's *really* nice to have a bunch of heads looking at the *same*
>>> thing...
>>> knowing where each other's gaze/focus is drifting, etc.
>>>
>>> It's a LOT easier to do hardware reviews remotely.  Especially with
>>> interactive video, etc.  "Everything" is visible at one time -- to
>>> everyone, etc.
>>
>> These days hardware *is* software.  The last company I worked for had
>> the FPGA
>> people in the software department.
>
> You can far more easily represent a piece of hardware on a "unified
> drawing"
> than you can with software.  So, you can provide a "block/functional
> diagram"
> that allows everyone to see the overall structure of a circuit; then, delve
> into selected "subsections" as appropriate.

As a general rule this is nonsense.  Drawings are great for the things 
drawing are good for.  But when a system gets very complex you can't put 
enough detail in a top level drawing for it to be worth a damn.  The 
decomposition is important.

There is nothing wrong with drawings.  They are great where they work. 
But to say that means it has to be hardware is silly.  Software can 
utilize drawings as well.


> Tools (and "thought processes") for doing this with a piece of software
> just don't exist in widespread use.

More nonsense.  I have personally used a number of such tools.


> This is why software is considerably harder to document, describe, analyze
> (in an isolated human brain), etc.

Unsubstantiated nonesense.

> *When* adequate tools (and methodologies) exist to provide (and are
> adopted)
> these same sorts of "illustrations" that allow entire software systems
> to be
> "conceptualized" in such a terse, abstract manner, I suspect there will be
> a quantum leap in software reliability, etc.

Only *simple* things can be abstracted simply.  It has nothing to do 
with hardware vs. software.


>      "Why is this module talking to this *other* module?  Is that
> intentional?
>      It *smells* like a potential problem area... let's take a look inside
>      those two modules!"

What does this have to do with hardware vs. software?  Both hardware and 
software have modules.  I can't even tell which this is about!

-- 

Rick

[toc] | [prev] | [next] | [standalone]

#18279

On 9/21/2014 4:02 PM, Clifford Heath wrote:
> On 22/09/14 07:23, Tim Wescott wrote:
>> See if YOU can find the error!
>> if (_port.elapsed() > 0 &&
>>     _port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>
> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
> Do I get a cookie?

Actually, there are more fundamental bugs.  Was the OP's intent to invoke
the "elapsed" method on "port" *once* or *twice*?  Or, twice iff the
first invocation yields 0?  Side-effects?  etc.

> This is why we should be using a language with a proper type system.

[toc] | [prev] | [next] | [standalone]

#18283

On 9/22/2014 4:15 PM, Don Y wrote:
> On 9/21/2014 4:02 PM, Clifford Heath wrote:
>> On 22/09/14 07:23, Tim Wescott wrote:
>>> See if YOU can find the error!
>>> if (_port.elapsed() > 0 &&
>>>     _port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))
>>
>> 0 and 1 will always be less than ((_port.clockHz() * 50LL) / 1000000)
>> Do I get a cookie?
>
> Actually, there are more fundamental bugs.  Was the OP's intent to invoke
> the "elapsed" method on "port" *once* or *twice*?  Or, twice iff the
> first invocation yields 0?  Side-effects?  etc.

Grrr... s.b., "!0".  <frown>  Too early in the day...

>> This is why we should be using a language with a proper type system.
>

[toc] | [prev] | [next] | [standalone]

#18227

On 21/09/14 23:23, Tim Wescott wrote:
> I just had to share this, god knows why.  I keep thinking that I want to 
> figure out a way to gather up a group of five or ten embedded software 
> consultants in my area and form a code review exchange, to keep things 
> like this from happening.  See if YOU can find the error!
> 
> I'm not sure why the Gnu compiler bitches if I compare an int to an 
> unsigned int, but doesn't bitch when I compare a boolean to an int -- but 
> there you are.
> 

As always in the software world, "we'll fix that in the next version" :

<https://gcc.gnu.org/gcc-5/changes.html>


-Wbool-compare (activated by -Wall) should spot that, but unless you
like the bleeding edge, you'll probably want to wait for the the stable
release.

[toc] | [prev] | [next] | [standalone]

#18254

> I'm not sure why the Gnu compiler bitches if I compare an int to an
> unsigned int,

Because that has partly unspecified results: what is the result of 
comparing -1 to 65535U on a 16-bit twos-complement machine? A 32-bit 
one? And how is the compiler to assume that's what you wanted?

> but doesn't bitch when I compare a boolean to an int

It wouldn't have anything to complain about: that has a perfectly 
well-defined result.

> if (_port.elapsed() > 0 &&
> 	_port.elapsed() < ((_port.clockHz() * 50LL) / 1000000))

Any self-respecting code reviewer still awake at this point of the 
procedure would have rapped you over the knuckles here for using a 
boolean to do what is rather obviously non-boolean work.

But since this is a _design_ error, not a _coding_ mistake, it's a bit 
too much to expect a compiler to warn you about it.  Mind-reading still 
isn't in the requirements spec for a compiler.

But I believe a properly tuned Lint could have caught this (particularly 
if it's been set to enforce MISRA C, which does frown upon such things).

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.arch.embedded

csiph-web