Hamish wrote:
> > ie how to trigger the crash and why does it happen?
Radim:
> Array was not zero terminated.
....
> The same problem in all versions.
>
> > should the real fix be "alloc += GMAPSET_MAX"?
>
> I dont think so, dynamic allocation is used.
ok, right. G_store() calls G_malloc() to hold the actual mapset name
using dynamic length.
> > or to modify/fix the header comments to say that the pointer will
> > be null if the location dir can't be opened or no mapsets are found?
>
> No, it says 'zero terminated array' so it should not be changed now so
> that it can return NULL, I think.
but before your patch it *did* return NULL if the location dir was not
found, or no viable mapsets were found. So the code and the header docs
are in disagreement, and perhaps somewhere in the code expects to test for
that NULL?
if it is allowed to return null, then there is no need to init the alloc
to 50 until the original n+2>alloc test later on.
> > and was the final null termination missing?;
>
> Yes.
> It seems that until now allocated space was inited to 0
> also by malloc, is it possible?
I believe that is only the case if you compile with the -g flag (at least
on Debian). So anyone building with debug flags won't catch missing null
terminators. (but debuggers won't have to deal with stray noise either)
the final mapset's name-string will always be null terminated by the
strcpy() in G_store(), so there will always be a \0 at the end of the
actual data, but the **mapsets list would rely on the G_realloc() += 50
elements list to be filled with zeros. And since there is no associated
number_of_mapsets integer to test against, to find the end of the array
a bit of code would need to scan until it sees two consecutive nulls.
right?
Hamish
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
