Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Plugins (gforce): Fix GCC warnings #291

Merged
merged 3 commits into from
Jan 30, 2025
Merged

Plugins (gforce): Fix GCC warnings #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85f5133
Plugins (gforce): Fix GCC 'may be used uninitialized' warnings.
kaixiong Feb 24, 2023
b6d8b4c
Plugins (gforce): Fix type-punning errors.
kaixiong Jan 26, 2025
c11eaca
Plugins (Build): Build G-Force plugin only if floats are 32-bit.
kaixiong Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions libvisual-plugins/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ CHECK_FUNCTION_EXISTS(mmap HAVE_MMAP)
CHECK_FUNCTION_EXISTS(mremap HAVE_MREMAP)
#AC_FUNC_MMAP

INCLUDE(CheckTypeSize)

# Assembly
#AM_PROG_AS

Expand Down Expand Up @@ -215,6 +217,14 @@ IF(ENABLE_GOOM2K4)
ENDIF()
ENDIF()

IF(ENABLE_GFORCE)
CHECK_TYPE_SIZE(float CXX_FLOAT_SIZE LANGUAGE CXX)
IF(NOT CXX_FLOAT_SIZE EQUAL 4)
MESSAGE(WARNING "G-Force requires 32-bit floats.")
SET(ENABLE_GFORCE no)
ENDIF()
ENDIF()

IF(ENABLE_GSTREAMER)
PKG_CHECK_MODULES(GSTREAMER gstreamer-1.0>=${GST_REQUIRED_VERSION} IMPORTED_TARGET)
IF(NOT GSTREAMER_FOUND)
Expand Down
2 changes: 1 addition & 1 deletion libvisual-plugins/plugins/actor/gforce/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ SET(GFORCE_SOURCE_DIR ${PROJECT_SOURCE_DIR}/plugins/actor/gforce)
SET(GFORCE_DATA_DIR ${LV_PLUGIN_DATA_DIR}/actor/actor_gforce)

SET(GFORCE_COMPILE_DEFS UNIX_X _REENTRANT)
SET(GFORCE_COMPILE_OPTIONS -fno-strict-aliasing -Wno-unknown-warning-option -Wno-sometimes-uninitialized)
SET(GFORCE_COMPILE_OPTIONS -Werror)

# Required on GCC but not Clang for some reason.
IF(NOT MINGW)
Expand Down
20 changes: 17 additions & 3 deletions libvisual-plugins/plugins/actor/gforce/Common/GeneralTools/Headers/XFloatList.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,35 @@
// by Andrew O'Meara

#include "XPtrList.h"
#include <string.h>

class XLongList;

inline float void_ptr_to_float(const void* value)
{
float result;
memcpy( &result, &value, sizeof( result ) );
return result;
}

inline void* float_to_void_ptr(float value)
{
void* result = 0;
memcpy( &result, &value, sizeof( value ) );
return result;
}

class XFloatList {

public:
XFloatList( ListOrderingT inOrdering = cOrderNotImportant ); // See XPtrList.h for ListOrderingT choices

// See XPtrList.h for description of functions.
virtual long Add( float inNum ) { return mList.Add( *((void**) &inNum) ); }
virtual long Add( float inNum ) { return mList.Add( float_to_void_ptr( inNum ) ); }
virtual void Add( const XFloatList& inList ) { mList.Add( inList.mList ); }
virtual bool RemoveElement( long inIndex ) { return mList.RemoveElement( inIndex ); }
virtual void RemoveAll() { mList.RemoveAll(); }
virtual float Fetch( long inIndex ) { long t = (long) mList.Fetch( inIndex ); return *((float*) &t);}
virtual float Fetch( long inIndex ) { return void_ptr_to_float( mList.Fetch( inIndex ) ); }
virtual bool Fetch( long inIndex, float* ioPtrDest ) const { return mList.Fetch( inIndex, (void**)ioPtrDest ); }
virtual long Count() const { return mList.Count(); }

Expand All @@ -36,7 +50,7 @@ class XFloatList {
// Smoothes all the floats in this list
void GaussSmooth( float inSigma );

float operator[] ( const long inIndex ) { long t = (long) mList.Fetch( inIndex ); return *((float*) &t); }
float operator[] ( const long inIndex ) { return void_ptr_to_float(mList.Fetch( inIndex )); }

// Generic utility fcn to gauss-smooth a 1D curve.
static void GaussSmooth( float inSigma, long inN, float inSrceDest[] );
Expand Down
6 changes: 3 additions & 3 deletions libvisual-plugins/plugins/actor/gforce/Common/GeneralTools/XFloatList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void XFloatList::FindMeans( long inNumMeans, float outMeans[], float inSigmaScal

// If this a local max. (Note: this could/should be improved for neighbors that are equal)
if ( ( cen > left && cen >= right ) ) {
sepCandidates.Put( i, *((void**) &cen) );
sepCandidates.Put( i, float_to_void_ptr( cen ) );
}
}

Expand Down Expand Up @@ -284,7 +284,7 @@ void XFloatList::Rank( XLongList& outRank, long inNumToRank ) const {


int XFloatList::sQSFloatComparitor( const void* inA, const void* inB ) {
float diff = *((float*) inB) - *((float*) inA);
float diff = void_ptr_to_float(inB) - void_ptr_to_float(inA);
if ( diff > 0.0 )
return 1;
else if ( diff < 0.0 )
Expand All @@ -296,7 +296,7 @@ int XFloatList::sQSFloatComparitor( const void* inA, const void* inB ) {


int XFloatList::sFloatComparitor( const void* inA, const void* inB ) {
float diff = *((float*) &inB) - *((float*) &inA);
float diff = void_ptr_to_float(inA) - void_ptr_to_float(inB);
if ( diff > 0.0 )
return 1;
else if ( diff < 0.0 )
Expand Down
11 changes: 5 additions & 6 deletions libvisual-plugins/plugins/actor/gforce/Common/GeneralTools/XStrList.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,12 @@ long XStrList::Add( const UtilStr& inStr ) {


long XStrList::FetchBestMatch( const UtilStr& inStr ) {
long best, bestScore, score, i;
UtilStr* str;

best = 0;
long bestScore = 0;
long best = 0;
UtilStr* str;

for ( i = 1; mStrings.Fetch( i, (void**) &str ); i++ ) {
score = str -> LCSMatchScore( inStr );
for ( long i = 1; mStrings.Fetch( i, (void**) &str ); i++ ) {
long score = str -> LCSMatchScore( inStr );
if ( score > bestScore || i == 1 ) {
best = i;
bestScore = score;
Expand Down
2 changes: 1 addition & 1 deletion libvisual-plugins/plugins/actor/gforce/Common/UI/LineXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
halfW = ( tw ) >> 1;

if ( tw < 12 ) {
const char* c_shape;
const char* c_shape = nullptr;
__circ( tw, c_shape )
for ( j = 0; j < tw; j++ ) {
c_x = c_shape[ j ];
Expand Down
24 changes: 18 additions & 6 deletions libvisual-plugins/plugins/actor/gforce/Common/io/CEgIStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "CEgIStream.h"

//#include <string.h>
#include <stdint.h>
#include <string.h>


UtilStr CEgIStream::sTemp;
Expand Down Expand Up @@ -32,8 +32,20 @@ long CEgIStream::GetLong() {


float CEgIStream::GetFloat() {
long v = GetLong();
return *( (float*) &v );
uint32_t c, n = GetByte();

c = GetByte();
n = n | ( c << 8 );

c = GetByte();
n = n | ( c << 16 );

c = GetByte();
n = n | ( c << 24 );

float result;
memcpy(&result, &n, sizeof(result));
Comment on lines +46 to +47
Copy link
Member

Choose a reason for hiding this comment

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

This implicitly relies on sizeof(float) == 4 and breaks if not, and would seem to break otherwise, at least in theory. I have a hard time finding guarantees for float being 4 bytes in size on all platforms. Is status quo good enough? Should we add a size-related assertion?

Copy link
Member Author

@kaixiong kaixiong Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a good point, I've always thought this was guaranteed by the C/C++ standard. (And it looks like we can't even be sure it's IEEE754..). Having some sort of check is desirable, is it better do it here or more globally? I think the assumption is pervasive in the code base. What are your thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

@kaixiong I think I'd favor local changes over global ones because that part of the code base is not in active development.

return result;
}


Expand All @@ -50,7 +62,7 @@ short int CEgIStream::GetShort() {


unsigned char CEgIStream::GetByte() {
unsigned char c;
unsigned char c = 0;

if ( mIsTied ) {
if ( mPos != 0 ) {
Expand All @@ -74,7 +86,7 @@ unsigned char CEgIStream::GetByte() {


unsigned char CEgIStream::PeekByte() {
unsigned char c;
unsigned char c = 0;

if ( mIsTied ) {
if ( mPos != 0 )
Expand Down
12 changes: 9 additions & 3 deletions libvisual-plugins/plugins/actor/gforce/Common/math/ExprVirtualMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#include <libvisual/libvisual.h>
#include <limits>
#include <stdlib.h>

#include <string.h>

#define __addInst( opcode, data16 ) long op = (opcode) | (data16); \
mProgram.Append( &op, sizeof(long) );
Expand Down Expand Up @@ -85,8 +85,8 @@ ExprUserFcn ExprVirtualMachine::sZeroFcn = { 0, { 0 } };
case cABS: r = fabs( r ); break; \
case cSIN: r = sin( r ); break; \
case cCOS: r = cos( r ); break; \
case cSEED: i = *((long*) &r); \
size = i % 31; \
case cSEED: i = pun_float_to_int32( r ); \
size = i % 31; \
srand( ( i << size ) | ( i >> ( 32 - size ) ) ); break; \
case cTAN: r = tan( r ); break; \
case cSGN: r = ( r >= 0 ) ? 1 : -1; break; \
Expand Down Expand Up @@ -114,6 +114,12 @@ ExprUserFcn ExprVirtualMachine::sZeroFcn = { 0, { 0 } };
case '%': { long tt = r2; r1 = (tt != 0) ? (( (long) r1 ) % tt) : 0.0; break; } \
}

int32_t pun_float_to_int32(float x) {
int32_t result = 0;
memcpy( &result, &x, sizeof( result ) );
return result;
}

ExprVirtualMachine::ExprVirtualMachine() {
mPCStart = 0;
mPCEnd = 0;
Expand Down
8 changes: 5 additions & 3 deletions libvisual-plugins/plugins/actor/gforce/GForceCommon/GF_Palette.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ void GF_Palette::Assign( const ArgList& inArgs ) {


void GF_Palette::Evaluate( PixPalEntry outPalette[ 256 ] ) {
int i;
float H, S, V, inc = 1.0 / 255.0;
float H = 0.0;
float S = 0.0;
float V = 0.0;
float inc = 1.0 / 255.0;

*mIntensity = 0;

Expand All @@ -50,7 +52,7 @@ void GF_Palette::Evaluate( PixPalEntry outPalette[ 256 ] ) {
if ( ! mS_I_Dep ) S = mS.Evaluate();
if ( ! mV_I_Dep ) V = mV.Evaluate();

for ( i = 0; i < 256; i++, *mIntensity += inc ) {
for ( int i = 0; i < 256; i++, *mIntensity += inc ) {

// Don't reevaluate vars that are indep of i
if ( mH_I_Dep ) H = mH.Evaluate();
Expand Down