Error building SDEmulator.cpp on OSX

[ghost]

=================================
Building Debug...
Platform: Unix-MACOSX
=================================
g++ -c SDEmulator.cpp -o Debug/SDEmulator.o -I/usr/local/include/SDL2 -D_THREAD_SAFE -DMACOSX -D_GNU_SOURCE=1 -DGUI=1 -DJOY_ANALOG_DEADZONE=8192 -g -MD -MP -MF Debug/SDEmulator.d -DUSE_SPI_DEBUG=1 -DUSE_EEPROM_DEBUG=1 -DUSE_GDBSERVER_DEBUG=1
SDEmulator.cpp:139:47: warning: format specifies type 'long' but the argument
      has type 'off_t' (aka 'long long') [-Wformat]
                        printf("\t%d: %s:%ld\n", i, entry->d_name, st.st_size);
                                         ~~~                       ^~~~~~~~~~
                                         %lld
SDEmulator.cpp:203:28: error: ordered comparison between pointer and zero
      ('FILE *' (aka '__sFILE *') and 'int')
                if (lastfile == -1 || fp <= 0) { *ptr++ = 0; }
                                      ~~ ^  ~
1 warning and 1 error generated.

[ry755]

I get that error too.

[ghost]

This can be fixed by changing if (lastfile == -1 || fp <= 0) { *ptr++ = 0; } to if (lastfile == -1 || fp <= (void *)0) { *ptr++ = 0; } in SDEmulator.cpp on line 203.

I'll submit a pull request shortly.

[Jubatian]

It still doesn't seem too nice for me as that condition there is intended to be a NULL test (the <= comparison is also slightly fishy on a pointer, the < part makes it smelling undefined behaviour). I would rather change the line as follows:

if ((lastfile == -1) || (fp == NULL)) { *ptr++ = 0; }

Also on line 153 the initialisation would read better as:

static FILE *fp=NULL;

Do these changes work on your system? (I don't see any problems when building and running this, the most likely one might be NULL being not defined, but that shouldn't happen) If it does, I would rather like to fix it this way.

(I reopened the ticket, please keep it open until the change is merged in)

[ghost]

I made the changes but I'm still receiving this error.

SDEmulator.cpp:203:28: error: ordered comparison between pointer and zero
      ('FILE *' (aka '__sFILE *') and 'long')
                if (lastfile == -1 || fp <= NULL) { *ptr++ = 0; }
                                      ~~ ^  ~~~~

[Jubatian]

Why are you still using <=? For me that smells undefined behaviour, I am not exactly sure, but <, <=, > and >= I think are only defined among pointers pointing into the same array or structure. Please try the code I posted.

According to the answer of this StackOverflow question, this use of the <= operator is most likely undefined: https://stackoverflow.com/questions/24807453/less-than-comparison-for-void-pointers (the answer refers the C11 standard, older versions shared the same). If it is undefined, then well, anything can happen.

[ghost]

Sorry, I missed the ==. That fixed it on my end, I'll update the PR.

[Jubatian]

I merged it in. Somehow I messed up the commit summary though (I wanted to stick to the usual convention of starting it as "Bugfix: (...)", and possibly missed a scrollbar). Anyway, it is fixed, so now this can be closed.