Re: [Gluster-devel] Query regarding GF_ASSERT macro

[Xavier Hernandez]

El 03/01/14 07:02, Harshavardhana ha escrit:
> > assert() macro also behaves in the same manner. It is an usual practice to
> > have assert() be a NOOP in production builds. assert()'s normal usage is to
> > assert that the expression is TRUE and it _should_ not be considered as a
> > replacement for NULL pointer checks. If we want to avoid NULL pointer
> > dereferences, it is a good idea to handle such scenarios and bail out
> > gracefully.
> > > A typical example of this scenario is :
> > >
> > >
> > > GF_ASSERT (shandle);
> > > GF_ASSERT (shandle->path);
> > >
> > > My question is why don't we abort in case of production build? Any
> > > specific reason?
> This question has come before and many times clang checks and other
> debugging tools "warn" about NULL deference issues after this macro
> has been invoked.
>
> But from what i gather GF_ASSERT in GlusterFS context is much like
> BUG_ON for Linux kernel - assert is only necessary during debugging -
> a backtrace is valid in case of a crash for production where we get
> something called gluster dump synonymous with 'Kernel dump'
Probably it's only a matter of semantics, but I think that an assert 
should be something that must be true under any circumstance. Even more, 
many times the program will crash soon if an assert fails and it's not 
handled (i.e. program aborted). If the program does not crash, something 
may get corrupted which is worse. If some condition can be false under 
some rare circumstances or it's possible to recover from a failed test, 
it would be better to use GF_VALIDATE_xxx macros.

I agree that on production builds, where (most) bugs have been killed, 
these macros could be a NOOP for performance reasons, however 
GF_ASSERT() checks the condition even in production builds. For this 
reason I think it would be better to force the program to abort even in 
production, or the macro be completely converted to a NOOP.

> I guess we can have much more broader discussion on its implications,
> FWIW it was necessary from GlusterFS context to not have "assert()"
> with in that macro.
>
> _______________________________________________
> Gluster-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/gluster-devel