search

cspiegel / terps

4 stars 2 forks source link

AdvSys won't build #13

Closed cibersheep closed 2 years ago

cibersheep commented 2 years ago

I forgot about this: Building with remglk but I guess with any implementation, AdvSys won't success (it misses size_t definition in glkstat.h)

[100%] Building C object CMakeFiles/advsys.dir/advsys/glkstart.c.o
In file included from /home/cibersheep/Escriptori/esborra/terps-upstream/terps/advsys/glkstart.c:14:
/home/cibersheep/Escriptori/esborra/terps-upstream/terps/remglk/glkstart.h:75:113: error: unknown type name ‘size_t’
   75 | _serialize_context_t, char *, glkunix_serialize_object_f, int, size_t, void *);
      |                                                                ^~~~~~
cibersheep commented 2 years ago

It looks like this might be it: https://github.com/cspiegel/terps/blob/master/advsys/glkstart.c#L15 I have questions @cspiegel

cspiegel commented 2 years ago

This looks to be a buglet in remglk: it uses size_t, so it really ought to be including stddef.h.

Including "header.h" before the other two, will define cstdio first, and fix the problem (I think). Would that be a valid fix?

Since this is (in my opinion) something that should be fixed in remglk, the proper fix is a pull request to Zarf including stddef.h. If he rejects that, then we can look at a workaround, but I'd only want to use that as a last resort. Basically, if a file uses a type, it needs to ensure that type is defined somewhere.

Also, removing https://github.com/cspiegel/terps/blob/master/advsys/glkstart.c#L13,L14 as they are already include in the "header.h". Is better to include them into a ifndef?

There's no harm in including the same file multiple times, assuming it's properly written (which Zarf's glk.h and glkstart.h are). Since glkstart.c directly uses things from both glk.h and glkstart.h, it should include them.

cibersheep commented 2 years ago

Ah, I understand. MR pushed to remglk. I'll close this for now