Are code assertions considered harmful?

[Andrew Bartlett]

On Sat, 2003-11-08 at 16:34, Cameron Paine wrote:
> I'm interested in opinions from a group of seasoned designers and
> coders...
> 
> I'm hunting some undocumented features in libsmbclient that are really
> laying my project's financial and time lines to waste. I guess I should
> have done a thorough code review before I committed to using the library
> so I guess that after 25 years at this I'm still able to be ambushed. ;-)

Are you using the exposed interfaces in libsmbclient.h, or actual
internal Samba library calls?

> Anyway, I'm trying to improve the worst excesses of the code and am
> being hamstrung by the paucity of documentation. It seems that the
> interface contracts for most of the functions in the library--both
> internal and exported--are seldom written down. I guess those of you
> who have worked on this code are in physical proximity and/or have
> developed good interpersonal communication and the nod and wink
> approach has reduced the time to implement. However as a latecomer
> who wants to: a) get his project delivered thereby reducing his
> financial exposure and; b) return the fruits of this labour to the
> community, the opacity of some of the contracts is really taking a
> toll.

For the interfaces exposed in libsmbclient.h, there are some Doxygen
comments.  But I certainly agree with your points on 'contracts' - we
basicly don't have any.   But if you do violate the interface contracts,
your behaviour is simply 'undefined...'

> To my question then: why are assertions considered unhelpful? I have
> found none so far and several of the thorny features I've found
> would have been detected if the original coder had stated his
> assumptions, as code, using assert() or an equivalent. Twice I've
> been tempted to put a couple in but I've baulked because this does
> not seem to be the "Samba Way".

There is SMB_ASSERT(), used in some places where we have tried to force
a particular interface, or (more commonly) where we need to bail out in
the case of internal logic errors.  

> Also, I understand the traditional (and lame IMO) argument for not
> documenting code. And yes, I can read C better than I can read my
> own handwriting. However, when the code implements conflicting
> behaviours, how can the maintainer know which was the intended one
> and which one slipped through the cracks in the testing regime.
> 
> While on the subject of code commentary, what is the attitude of
> the team to others adding commentary to their code? So far I've
> amassed a dozen pages of explanatory notes that I'm keeping
> separate from the code. I'm doing this primarily so I don't
> pollute the code with (possibly incorrect) statements about
> intended behaviour. I could mitigate against this by documenting
> what the code does (not what people seem to assume that it does)
> but that may then create conflicts with the small amount of doco
> that does exist.

There are places in Samba where the effort has been taken to create (and
in fewer places, maintain) Doxygen style documentation.  The problem is
very much a matter of the chicken and the egg - the documented code is
certainly not the majority, and I certainly don't read the generated
result.  This means that there isn't the push to grow it over the entire
source tree.  

For Samba4, Tridge is trying to document the code he is writing, and
push a level of required documentation onto the whole team as we start
coding on it.  I really do hope this works.

> I'd really appreciate your thoughts and your guidance.
> 
> Cameron Paine

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20031108/8f453a69/attachment.bin