Skip to content

Add missing include#849

Merged
vapier merged 1 commit intolibgd:masterfrom
dg0yt:fix-msvc-webp
Oct 30, 2022
Merged

Add missing include#849
vapier merged 1 commit intolibgd:masterfrom
dg0yt:fix-msvc-webp

Conversation

@dg0yt
Copy link
Copy Markdown
Contributor

@dg0yt dg0yt commented Oct 30, 2022

gd_webp.c needs ssize_t which is defined in gd_intern.h for MSVC.
(Other C files which use this type include this header, too.)

Fixes this error:

FAILED: src/CMakeFiles/libgd_static.dir/gd_webp.c.obj 
C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1433~1.316\bin\Hostx64\x64\cl.exe   -DBGDWIN32 -DHAVE_CONFIG_H -DMSWIN32 -DNONDLL=1 -DWIN32 -DWINVER=0x0500 -D_CRT_SECURE_NO_DEPRECATE -D_WIN32 -D_WIN32_IE=0x0600 -D_WIN32_WINNT=0x0500 -ID:\buildtrees\libgd\src\1819764bdc-319f487d20.clean\src -ID:\buildtrees\libgd\x64-windows-static-dbg -ID:\buildtrees\libgd\x64-windows-static-dbg\src -ID:\buildtrees\libgd\src\1819764bdc-319f487d20.clean\before -external:ID:\installed\x64-windows-static\include -external:W0 /nologo /DWIN32 /D_WINDOWS /W3 /utf-8 /MP  /D_DEBUG /MTd /Z7 /Ob0 /Od /RTC1 /showIncludes /Fosrc\CMakeFiles\libgd_static.dir\gd_webp.c.obj /Fdsrc\CMakeFiles\libgd_static.dir\libgd_static.pdb /FS -c D:\buildtrees\libgd\src\1819764bdc-319f487d20.clean\src\gd_webp.c
D:\buildtrees\libgd\src\1819764bdc-319f487d20.clean\src\gd_webp.c(108): error C2065: 'ssize_t': undeclared identifier

`gd_webp.c` needs `ssize_t` which is defined in `gd_intern.h` for MSVC.
Copy link
Copy Markdown
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you! This looks good to me, although I'm not convinced that it is a good idea to provide these fallback definitions in gh_intern.h in the first place.

I'm not sure, though, if this shouldn't better target GD-2.3 @vapier, thoughts?

@vapier
Copy link
Copy Markdown
Member

vapier commented Oct 30, 2022

gd_intern.h is the best place we have for these things. the rest of the gd codebase should be able to assume a recent POSIX environment and completely basic types like ssize_t. so either we drop MSVC, or we workaround their crap setup somewhere. afaik, MSVC refuses to provide a standard ssize_t and such which is why we have these defines.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Oct 30, 2022

gd_intern.h is the best place we have for these things.

I'd prefer to have such fallback definitions in a generated config.h or such, but I agree that having them in gd_intern.h is better than to spread them all over the codebase.

@vapier
Copy link
Copy Markdown
Member

vapier commented Oct 30, 2022

the MSVC logic isn't really generated in this case ... it's hand-written stuff. it also requires including other headers in order to make the types work correctly (which we def don't want in config.h). even though the exact line is a define (#define ssize_t SSIZE_T), and is thus fine to setup in config.h, we have to check a couple diff headers in order to find SSIZE_T for MSVC, and that wouldn't work in config.h, and i don't want to include that boiler plate in every header file.

GNU projects in general rely on the gnulib project to hide this stuff, and it can go as far as generating local headers to overlay/replace system headers (e.g. sys/types.h). the downside is that gnulib is (L)GPL leaning while gd is BSD-like, and having us depend on gnulib will make the combined work subject to the (L)GPL (which personally i don't care about, but it seems an unreasonable change for the gd project). plus, gnulib really really wants to be linked by itself into a shared library or executable and not expose its symbols, which means putting it into a static libgd.a will easily cause problems for users.

i'm not aware of any other portability shims out there which means we're left open-coding our own ad-hoc ones like gd_intern.h. which is a fun name like: let the interns deal with this internal crap :p. on the flip side, it's a bit of pressure to try and keep our C library footprint somewhat smaller, which can help some embedded folks. shrug

@vapier vapier merged commit fe3e0d3 into libgd:master Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants