Re: [PATCH 0/3] isdn: patches for 2.6.31

[Tilman Schmidt <tilman@imap.cc>]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On 01.06.2009 12:04, David Miller wrote:
> First problem in the second patch.  You're doing two things
> at once.  You're adding function documentation and also adding
> a NULL pointer check.

No problem. I can split the patch in two if you prefer it that way.

> Second problem, the NULL pointer check is gratuitous.  Document
> that the 'm' member has to be non-NULL and leave the check out.

That would be a bad solution for two reasons:
First, the 'm' member is private to capiutil.{c,h}. Callers are
not supposed to access it. Therefore it shouldn't be referred to
in the interface documentation. At best, such a mention would
leave users of the function confused how to assure that condition.
At worst, it might mislead them into meddling directly with the
member, thereby producing incorrect code.
And second, the main use of capi_cmsg2str() is for error reporting
and debugging output. Oopsing in an error handler is particularly
troublesome. At the same time, the risk of the 'm' member being
unexpectedly NULL is particularly high when something has gone
wrong already. So a safety check is advisable in this case.

Thanks,
Tilman

PS: Any objections against the other two patches?

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]