Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
define customized fluid_return_if_fail 
unlike glib's g_return_if_fail, dont log to console if condition fails
  • Loading branch information
@derselbst
derselbst committed Nov 7, 2017
1 parent 1b2a098 commit b1d0db7
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions src/utils/fluid_sys.h
View file
Expand Up @@ -56,8 +56,6 @@ void fluid_time_config(void);

/* Misc */

#define fluid_return_val_if_fail g_return_val_if_fail
#define fluid_return_if_fail g_return_if_fail
#define FLUID_INLINE inline
#define FLUID_POINTER_TO_UINT GPOINTER_TO_UINT
#define FLUID_UINT_TO_POINTER GUINT_TO_POINTER
Expand All @@ -70,6 +68,15 @@ void fluid_time_config(void);
#define FLUID_LE32TOH(x) GINT32_FROM_LE(x)
#define FLUID_LE16TOH(x) GINT16_FROM_LE(x)


#define fluid_return_val_if_fail(cond, val) \
if(cond) \
; \
else \
return val

#define fluid_return_if_fail(cond) fluid_return_val_if_fail(cond, ((void)(0)))

/*
* Utility functions
*/
Expand Down

6 comments on commit b1d0db7

@carlo-bramini
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix breaks compilation with Visual Studio.
The macro fluid_return_if_fail() should be changed to:

#define fluid_return_if_fail(cond) if (!(cond)) return

Because fluid_return_val_if_fail(cond, ((void)(0))) expands a "return (void)0" and MSVC complains that a void function is returning a value.

@derselbst
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the hint. I tested it with clang and gcc, both accept it. And IMO MSVC is wrong. I'm returning a value that I've casted to void. Thus I'm returning void. I admit it's an ugly and very nit-picky way of avoiding code duplications, but I dont see the problem.

Because fluid_return_val_if_fail(cond, ((void)(0))) expands a "return (void)0"

Shouldnt it expand to "return ((void)(0))"? What version(s) of MSVC have you tried?

@mawe42
Copy link
Member

@mawe42 mawe42 commented on b1d0db7 Nov 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just compiled FluidSynth on Windows 10 64-bit with Visual Studio 2017 64-bit to test LADSPA on Windows and got the same warnings.

@mawe42
Copy link
Member

@mawe42 mawe42 commented on b1d0db7 Nov 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify: this doesn't seem to break the build, but just throws warnings all over the place. But there is probably a switch to turn warnings into errors, just like on Linux gcc...

@mawe42
Copy link
Member

@mawe42 mawe42 commented on b1d0db7 Nov 12, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit it's an ugly and very nit-picky way of avoiding code duplications, but I dont see the problem.

If it's ugly and only saves three extra code lines, maybe it's not really worth it :-)

@derselbst
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious, grabbed a VS2012 and compiled a simple demo prog. I didnt got a warning, probably used a too low warning level. But at least it compiled. The assembly output is also the same for both return and return ((void)0). I really think that in this case raising a warning is incorrect. Casting a value to void means discarding the value and in the end nothing is returned. Disabling this warning is not an option, so I'll be so kind and change this to a conventional boring return.

Please sign in to comment.