Re: About GF_ASSERT() macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



As per your review comments, introduced GF_ABORT as part of patch https://review.gluster.org/#/c/18309/5

I wouldn't get into changing anything with ASSERT at the moment, as there are around ~2800 instances :-o

Wherever it is critical, lets call 'GF_ABORT()' in future, and also we should have a 'checkpatches.pl' check to warn people about usage of GF_ABORT().

Regards,
Amar

On Fri, Nov 3, 2017 at 2:05 PM, Xavi Hernandez <jahernan@xxxxxxxxxx> wrote:
Hi all,

I've seen that GF_ASSERT() macro is defined in different ways depending on if we are building in debug mode or not.

In debug mode, it's an alias of assert(), but in non-debug mode it simply logs an error message and continues.

I think that an assert should be a critical check that should always be true, specially in production code. Allowing the program to continue after a failure on one of these checks is dangerous. Most probably it will crash later, losing some information about the real cause of the error. But even if it doesn't crash, some internal data will be invalid, leading to a bad behavior.

I think we should always terminate the process if an assertion fails, even in production level code. If some failure is not considered so much critical by the coder, it should not use GF_ASSERT() and only write a log message or use another of the condition check macros.

Thoughts ?

Xavi

_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://lists.gluster.org/mailman/listinfo/gluster-devel



--
Amar Tumballi (amarts)
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://lists.gluster.org/mailman/listinfo/gluster-devel

[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux