Skip to content

libgis: G_vasprintf bug fix#3650

Open
HuidaeCho wants to merge 6 commits into
OSGeo:mainfrom
HuidaeCho:G_vasprintf_bugfix
Open

libgis: G_vasprintf bug fix#3650
HuidaeCho wants to merge 6 commits into
OSGeo:mainfrom
HuidaeCho:G_vasprintf_bugfix

Conversation

@HuidaeCho

@HuidaeCho HuidaeCho commented Apr 23, 2024

Copy link
Copy Markdown
Member

This PR fixes a G_vasprintf bug for #else of #ifdef HAVE_ASPRINTF.

From the man page of vsnprintf():

   Upon successful return, these functions return  the  number  of  bytes
   printed (excluding the null byte used to end output to strings).

   The  functions  snprintf() and vsnprintf() do not write more than size
   bytes (including the terminating null byte ('\0')).  If the output was
   truncated due to this limit, then the return value is  the  number  of
   characters (excluding the terminating null byte) which would have been
   written to the final string if enough space had been available.  Thus,
   a  return  value  of size or more means that the output was truncated.
  1. We don't need to keep doubling size *= 2 because count is already the full length without \0 if it's greater than or equal to size.
  2. ap cannot be reused in a for loop. It needs to be copied before its first use so we can use it again.

@github-actions github-actions Bot added C Related code is in C libraries labels Apr 23, 2024
@HuidaeCho HuidaeCho added the bug Something isn't working label Apr 23, 2024
@echoix echoix added this to the 8.4.0 milestone Apr 23, 2024
@HuidaeCho HuidaeCho self-assigned this May 9, 2024
@wenzeslaus wenzeslaus modified the milestones: 8.4.0, 8.4.1 Jun 10, 2024
@echoix echoix requested a review from nilason October 21, 2024 19:51
@echoix

echoix commented Oct 21, 2024

Copy link
Copy Markdown
Member

@nilason What's your opinion on this?

It was fixing a bug 6 months ago caught in the cmake PR.

@wenzeslaus

Copy link
Copy Markdown
Member

The build errors were no longer visible, so I updated the branch again.

@wenzeslaus

Copy link
Copy Markdown
Member

The error is asprintf.c:107:15: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare].

Details:

2024-10-25T12:40:26.7331364Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include   -fopenmp -DGRASS_VERSION_DATE=\"'2024'\" -DPACKAGE=\""grasslibs"\"        -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/gis\" -o OBJ.x86_64-pc-linux-gnu/ascii_chk.o -c ascii_chk.c
2024-10-25T12:40:26.7571250Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include   -fopenmp -DGRASS_VERSION_DATE=\"'2024'\" -DPACKAGE=\""grasslibs"\"        -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/gis\" -o OBJ.x86_64-pc-linux-gnu/asprintf.o -c asprintf.c
2024-10-25T12:40:26.7801645Z asprintf.c: In function ‘G_rasprintf’:
2024-10-25T12:40:26.7803610Z asprintf.c:107:15: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
2024-10-25T12:40:26.7804989Z   107 |     if (count >= osize) {
2024-10-25T12:40:26.7805505Z       |               ^~
2024-10-25T12:40:26.7854808Z cc1: all warnings being treated as errors
2024-10-25T12:40:26.7868908Z make[3]: *** [../../include/Make/Compile.make:32: OBJ.x86_64-pc-linux-gnu/asprintf.o] Error 1
2024-10-25T12:40:26.7870116Z make[3]: Leaving directory '/home/runner/work/grass/grass/lib/gis'
2024-10-25T12:40:26.7922454Z make[3]: Entering directory '/home/runner/work/grass/grass/lib/proj'
2024-10-25T12:40:26.7923557Z test -d OBJ.x86_64-pc-linux-gnu || mkdir -p OBJ.x86_64-pc-linux-gnu
2024-10-25T12:40:26.7944649Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include    -I/usr/include/gdal -DPACKAGE=\""grasslibs"\"   -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/proj\" -o OBJ.x86_64-pc-linux-gnu/convert.o -c convert.c
2024-10-25T12:40:26.8622582Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include    -I/usr/include/gdal -DPACKAGE=\""grasslibs"\"   -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/proj\" -o OBJ.x86_64-pc-linux-gnu/datum.o -c datum.c
2024-10-25T12:40:26.9104311Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include    -I/usr/include/gdal -DPACKAGE=\""grasslibs"\"   -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/proj\" -o OBJ.x86_64-pc-linux-gnu/do_proj.o -c do_proj.c
2024-10-25T12:40:26.9827074Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include    -I/usr/include/gdal -DPACKAGE=\""grasslibs"\"   -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/proj\" -o OBJ.x86_64-pc-linux-gnu/ellipse.o -c ellipse.c
2024-10-25T12:40:27.0308181Z gcc  -std=gnu17 -fPIC -Wall -Wextra -isystem/usr/include/gdal -Wpedantic -Werror  -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include    -I/usr/include/gdal -DPACKAGE=\""grasslibs"\"   -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -I/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/include -DRELDIR=\"lib/proj\" -o OBJ.x86_64-pc-linux-gnu/get_proj.o -c get_proj.c
2024-10-25T12:40:27.0830111Z gcc -shared -o /home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib/libgrass_gproj.8.5.so -L/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -L/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -Wl,--export-dynamic -Wl,-rpath-link,/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/lib -Wl,-rpath,/home/runner/install/grass85/lib -Wl,-soname,libgrass_gproj.8.5.so OBJ.x86_64-pc-linux-gnu/convert.o OBJ.x86_64-pc-linux-gnu/datum.o OBJ.x86_64-pc-linux-gnu/do_proj.o OBJ.x86_64-pc-linux-gnu/ellipse.o OBJ.x86_64-pc-linux-gnu/get_proj.o  -lgrass_gis.8.5 -L/usr/lib -lgdal -lproj  -lm -lm
2024-10-25T12:40:27.0874178Z /usr/bin/ld: cannot find -lgrass_gis.8.5: No such file or directory
2024-10-25T12:40:27.0974660Z collect2: error: ld returned 1 exit status

Comment thread lib/gis/asprintf.c
va_end(ap);

for (;;) {
if (count >= osize) {

@nilason nilason Oct 25, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This addresses the warning, and take into account if the previous vsnprintf call fails.

Suggested change
if (count >= osize) {
if (count < 0)
return count;
if ((size_t)count >= osize) {

@neteler neteler modified the milestones: 8.4.1, 8.5.0 Dec 25, 2024
@echoix

echoix commented Feb 18, 2025

Copy link
Copy Markdown
Member

Should we update the branch first?

@nilason

nilason commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

@HuidaeCho shall we push this for 8.5?

@HuidaeCho

Copy link
Copy Markdown
Member Author

@HuidaeCho shall we push this for 8.5?

Yes. Let's push it.

@nilason

nilason commented Feb 12, 2026

Copy link
Copy Markdown
Contributor

There was a compiler warning to which I had a suggestion. That warning should be addressed and let us update the branch before merge.

@HuidaeCho

Copy link
Copy Markdown
Member Author

There was a compiler warning to which I had a suggestion. That warning should be addressed and let us update the branch before merge.

I'll take a look into it this weekend.

@wenzeslaus wenzeslaus modified the milestones: 8.5.0, 8.5.1 May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working C Related code is in C libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants