Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't append garbage data to tag names #161

Closed

Conversation

TheDorkKnight
Copy link

Bug was introduced in 662729c

The string should not be terminated at the end of the entire shared buffer. Rather, it should be terminated at the index after the end of the copied string.

This commit restores the original behaviour before 662729c

Moving the index variable into the for-loop scope is a good practice in general, but it introduced a bug in this particular case.

Example of bug:

A key like:
addr:housenumber
becomes
addr_housenumber ¬’/
or something like that, since the bytes after 'r' are undefined until byte number 255.

Bug was introduced in OSGeo/gdal@662729c

The string should be terminated at the
end of the entire shared buffer. Rather,
it should be terminated at the index
after copying stops.

This commit restores the original
behaviour before 662729c

Moving the index variable into the for-loop
scope is a good practice in general, but
not in this particular case
@rouault
Copy link
Member

rouault commented Oct 20, 2016

@schwehr Looks related to one of your commits. Given the date the issue is probably in the 2.1 branch as well

@schwehr
Copy link
Member

schwehr commented Oct 21, 2016

@schwehr schwehr closed this Oct 21, 2016
rouault added a commit that referenced this pull request Oct 21, 2016
@TheDorkKnight
Copy link
Author

@schwehr No prob, anytime!

I work with GDAL a lot, so I'm aware of quite a few small issues like this. However, I haven't gotten around to submitting patches before, since it's a bit cumbersome to raise/follow issues in trac.

Anyway, hopefully I'll be able to submit plenty of fixes in the future!

@schwehr
Copy link
Member

schwehr commented Oct 21, 2016

@TheDorkKnight , Great! Looking forward to more patches.

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.

None yet

3 participants