[GRASS-dev] Re: [bug #1140] (grass) Non-portable snprintf() used in several programs

GRASS GIS GRASS GIS developer
1

Hello Markus!
I contend that most of those recent introductions of snprintf() probably don't need it. E.g. the first one (raster3d/r3.in.acii/main.c) can calculate the length of the string to be malloc'ed using strlen() so can guarantee to leave enough space. The second one (db/drivers/dbf/dbfexe.c) can use a fixed field width specifier for the %d format string to fix the length also - again predictable.

So I will work through those and see if I can change them not to use snprintf(). Started it now. I feel if there's something where the length of the resulting string really cannot be predicted, then we should use G_asprintf() because it's there. It does seem to be a matter of personal preference and philosophy though! But I think as Glynn has said before, most of the places snprintf() is used the return value is not checked. So if an overflow was prevented, and the string was not the expected value, the program would just ignore it!

So to summarise - G_snprintf() would be useful to have to easily fix the places that snprintf() was erroneously introduced - so you could put it in, but I think we should discourage its use in favour of calculating how long the string will be and allocating enough memory.

Paul

On Tue, 4 Jul 2006, Markus Neteler via RT wrote:

Hi,

the snprintf() is used as of today in:

./raster3d/r3.in.ascii/main.c

./db/drivers/dbf/dbfexe.c

./raster/r.support/front/front.c

./raster/r.support/front/check.c

./raster/r.support/front/run.c

./raster/r.support/modhead/check_un.c

./raster/r.support/modhead/modhead.c

./raster/r.support/modhead/ask_format.c

./lib/init/clean_temp.c

./lib/db/dbmi_client/select.c

./lib/gis/user_config.c

./lib/vector/dglib/examples/components.c

I suggest to implement G_snprintf() and use that

function everywhere. Then we can fix in a single place

if needed.

Markus

-------------------------------------------- Managed by Request Tracker

2

Hello Paul,

On Tue, Jul 04, 2006 at 02:08:48PM +0100, Paul Kelly wrote:

Hello Markus!
I contend that most of those recent introductions of snprintf() probably
don't need it. E.g. the first one (raster3d/r3.in.acii/main.c) can
calculate the length of the string to be malloc'ed using strlen() so can
guarantee to leave enough space. The second one (db/drivers/dbf/dbfexe.c)
can use a fixed field width specifier for the %d format string to fix the
length also - again predictable.

So I will work through those and see if I can change them not to use
snprintf(). Started it now. I feel if there's something where the length
of the resulting string really cannot be predicted, then we should use
G_asprintf() because it's there. It does seem to be a matter of personal
preference and philosophy though! But I think as Glynn has said before,
most of the places snprintf() is used the return value is not checked. So
if an overflow was prevented, and the string was not the expected value,
the program would just ignore it!

So to summarise - G_snprintf() would be useful to have to easily fix
the places that snprintf() was erroneously introduced - so you could put
it in, but I think we should discourage its use in favour of calculating
how long the string will be and allocating enough memory.

thanks for working on it.

I have submitted now the G_snprintf() wrapper function to
lib/gis/snprintf.c
with the discouragement included on top.

You may use it now if snprintf() cannot be avoided.

Markus

3

Paul Kelly wrote:

I contend that most of those recent introductions of snprintf() probably
don't need it. E.g. the first one (raster3d/r3.in.acii/main.c) can
calculate the length of the string to be malloc'ed using strlen() so can
guarantee to leave enough space. The second one (db/drivers/dbf/dbfexe.c)
can use a fixed field width specifier for the %d format string to fix the
length also - again predictable.

Field width specifiers only control the minimum width of a field (i.e.
the amount of padding or the number of leading zeroes); a field can
still be larger than the specified width.

For %d, it's reasonable to assume a 32-bit integer, which means that
it can never be more than 11 characters wide (10 digits, with a
leading minus if negative).

--
Glynn Clements <glynn@gclements.plus.com>

4

Glynn Clements wrote:

Paul Kelly wrote:

I contend that most of those recent introductions of snprintf() probably don't need it. E.g. the first one (raster3d/r3.in.acii/main.c) can calculate the length of the string to be malloc'ed using strlen() so can guarantee to leave enough space. The second one (db/drivers/dbf/dbfexe.c) can use a fixed field width specifier for the %d format string to fix the length also - again predictable.

Field width specifiers only control the minimum width of a field (i.e. the amount of padding or the number of leading zeroes); a field can
still be larger than the specified width.

Ah yes I realised that when I actually went to implement my proposed change.

For %d, it's reasonable to assume a 32-bit integer, which means that
it can never be more than 11 characters wide (10 digits, with a
leading minus if negative).

Well in this case the existing code was allocating 32 bytes of space and then using snprintf so I think it should be safe to change the snprintf a simple sprintf in this case (db/drivers/dbf/dbfexe.c).