> > From: Christophe de Dinechin <dinechin@xxxxxxxxxx> > > Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx> > --- > docs/spice_style.txt | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/docs/spice_style.txt b/docs/spice_style.txt > index 72ed2ef7..61cb0701 100644 > --- a/docs/spice_style.txt > +++ b/docs/spice_style.txt > @@ -82,7 +82,11 @@ Comments that are prefixed with `FIXME` describe a bug > that need to be fixed. Ge > ASSERT > ------ > > -Use it freely. ASSERT helps testing function arguments and function results > validity. ASSERT helps detecting bugs and improve code readability and > stability. > +Use assertions liberally. They helps testing function arguments and function > results validity. Assertions helps detecting bugs and improve code > readability and stability. > + > +Several form of assertion exist, notably: > +- spice_assert which should be preferred for any assertion related to SPICE > itself > +- glib asserts (many forms) which should be preferred for any assertion > related to the use of glib > > sizeof Yes, this section looks... weird. I think there is a problem here and this entire section is entirely broken now. To sum up: we don't use ASSERT! At all. But we use some kind of asserts! Yes, sounds confusing. But you have to consider the long story of this project (which unfortunately I don't know entirely by myself!). Maybe I'm a bit wrong but the project was in C++ using a lot of Windows knowledge and styles. This is a lot lost in the current code is is hard to see but I was also a Windows developer for quite some time (I don't regret it) and I can still note the smell of it! What's specifically "ASSERT" (not assert, not asserts) ? ASSERT is a Windows macro that aborts the code if the condition is false. It helps detecting the bugs but this macro is NEVER compiled in release code (Windows rules!). The difference seems small but is way different/ Is not used for runtime checks that can happen as this would probably causes crashes on release. Is used for checks that should never happen to help developers testing with debugging versions (on Windows you test with debugg versions). How can I say that? There are some places where this macro is too aggressive, where should be mathematically impossible to reach. For instance in common/ring.h they are used to check internal states at every low level operation (in functions that are inlined). I just think in the history of code ASSERT were changed to spice_assert. Other indications (looks like to be a paleontologist) are in code like common/quic.c (notably encode function) which are basically testing is a 32 bit integer has more that 32 bits! In our code we never disable our assert style asserts (either spice_assert or g_assert) which make them suitable for runtime checks but a bit overwhelming to check the "obvious". Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel