Proposed addition to CodingStyle web page about assert(a && b)

[Adam Richter]

On Sun, May 5, 2019 at 7:18 AM walter harms <wharms at bfs.de> wrote:
> Am 05.05.2019 09:11, schrieb Matthieu Herrb:
> > On Sat, May 04, 2019 at 03:47:31PM -0700, Adam Richter wrote:
> >> Hi, everyone.
> >>
> >> I would like to propose that whoever has the ability to edit the web
> >> page add a line like the following to
> >> https://www.x.org/wiki/CodingStyle/ :
> >>
> >> - Separate assert(a && b) into assert(a) and assert(b).
> >>
> >>
> >> Thanks in advance for any input on this.
> >
> > Hi,
> >
> > I'm not sure if this advice belongs to this wiki page which is more
> > oriented on the appearance of the code than on semantics or
> > development good practices.
> >
> > On the development good practices side, I think assert() should be
> > banned as much as possible form libraries and drivers.
> >
> > You don't know anything about the caller context and having it beeing
> > brutally abort()ing is brutal and my lead to security issues
> > (data leaks in the core file for instance) or data corruption.
> >
> > In libraries assert() should never be used to reject bad user input or
> > any other error condition that can happen for some known reason. It
> > should really only be used to document conditions that should really
> > never happen. In all other cases the function should be able to return
> > an error to the caller (which should of course not ignore them).
> >
> >
>
> i do not comment on the use of assert() generally, it can be used
> by anyone who likes that. Things are getting problematic when use
> like this:
>
>    assert(0 < asprintf(&lf, "%s/Library/Logs/X11", home));
>
> this is simply dangerous as you can define NDEBUG and let everything vanish.
>
> BTW are the libraries routinely compiled with NDEBUG enabled ?
>
> re,
>  wh
>
>
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel

Matthieu, thank you for pointing out that most of that CodingStyle web
page is about page layout.  The last entry on that page is about
program practices, but that seems to be an exceptional situation
calling for a reversal of a previous X development convention.  I did
not notice it before your comment, but I see now that that page really
is not a collection of practical semantics recommendations.  So,
unless anyone would like to advocate otherwise, I drop my request for
an edit of that CodingStyle page.

Matthieu and Walter, thank you for your input on the point the use of
assert.  I think I mostly agree with both of you.  Where I might
disagree ("might" because I don't know your views on this, so maybe we
already agree) is that I think that assert() is almost always much
better than doing nothing, so I would encourage people to write assert
statements first, if that's easier, and then revisit them to see which
assert statements can be converted to more controlled error handling.
By offering this opinion, I don't mean to imply that my opinion needs
to carry any weight; I'm just offering thoughts for you to consider.

With that said, I'm not quite sure what to do with my darwin.c changes
to replace some assert statements, including the one that Walter used
as an example, with code that calls FatalError() if the operation in
question fails, given that I have not easily found an OSX system on
which to compile it.  So, I'll ask now, would anyone object to my
submitting that darwin.c patch (darwin-assert-v2.diff earlier in this
thread) under this circumstance?  If nobody complains or requests I do
otherwise in the next ~24 hours, I guess I'll go ahead and submit that
patch on gitlab.freedesktop.org.

Adam