[GRASS-dev] Right way to use G_getl2

[Markus Metz]

I think the confusion arises because G_getl2(char *buf, int n, FILE *
fd) reads in at most n - 1 characters, not n characters. The next
character is always set to '\0' and guaranteed to be at most the n-th
character. That also means that G_getl() does not check if the buffer
is large enough to hold the line to be read, which it should, IMHO.

To the example with v.in.ascii:
The module first finds the longest line, the length including the
terminating character. This number is then passed to G_getl2() as n,
which means that the end of the line is not read for the longest line,
because the end of the line is the n-th character.

I would suggest to modify G_getl2() to read at most n (not n - 1)
characters and include a check if the buffer is long enough:

--->
Index: getl.c
===================================================================
--- getl.c    (revision 57537)
+++ getl.c    (working copy)
@@ -67,7 +67,7 @@
     int c;
     int ret = 1;

-    while (i < n - 1) {
+    while (i < n) {
     c = fgetc(fd);

     if (c == EOF) {
@@ -88,6 +88,11 @@
         }
         break;
     }
+
+    if (i == n - 1) {
+        G_warning("G_getl2(): buffer too small");
+        break;
+    }

     buf[i] = c;
<---

Markus M


On Fri, Sep 13, 2013 at 4:58 AM, Vaclav Petras <wenzeslaus at gmail.com> wrote:
> Hi,
>
> this relates to the recent fix for r.in.ascii (r57649). The skip parameter
> [2] required one line more because when reading lines using G_getl2 [3]
> function and counting these lines it counted one line more.
>
> The documentation of the function says that it reads max n-1 bytes. In other
> words, n is the size of the buffer, so it is clear that it can n-1 useful
> characters plus string terminating character '\0'. Quite standard way but...
>
> The problem in v.in.ascii was that it the caller passed the previously
> determined maximum number of characters on line in file plus one (space for
> the terminating character). So this number is the same for all lines and it
> works well for all lines except for the longest one.
>
> The reason is that G_getl2 stops reading characters for the longest line and
> leaves end line character in the stream. So, in fact, the longest line is in
> buffer and can be processed. However, the next line is than only the
> remaining end line character. This is not what the caller expected. There is
> even no way to find out that the line was not read completely.
>
> So, finally: What is the right usage of G_getl2? Should the caller use
> buffer size equal to maximum number of expected characters on line plus end
> of line character (plus 2 on MS Windows) plus terminating character? Or
> should he just pass the n parameter increased by one (two on MS Windows)
> since in fact nothing will be stored to buffer? Or should he just allocate
> really huge buffer and than read chars from buffer if he wants to store
> them?
>
> Thanks,
> Vaclav
>
> [1] https://trac.osgeo.org/grass/changeset/57649
> [2] http://grass.osgeo.org/grass70/manuals/v.in.ascii.html
> [3]
> http://grass.osgeo.org/programming7/getl_8c.html#a4bf86e666fda18059c42b0f5eca6f9bd
>
> Note 1: Don't be confused from documentation since the n parameter
> description for G_getl2 is wrong.
>
> Note 2: In description I was taking only about Unix line endings. On Windows
> we need two bytes more to store it as noted in questions. The case of old
> Mac OS is not cover because it is even more tricky in G_getl2 function.
>
>
> _______________________________________________
> grass-dev mailing list
> grass-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/grass-dev