giza_parse_string() fix, add cpgqinf() support, fix critical bug found doing this #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The meaning of the nGlyph index used in giza_parse_string changed subtly
during development and the handling of backspace was not changed
accordingly.
The result was that "x^2\b_i" rendered fine but the unit superscript over
the decimal point in PGTBOX() now looked horrible because the pen position
was moved to an erroneous position. Sigh.
In order to make "x-i-squared" render nicely it must now be spelled as:
x^2\b\b_i
with two backspaces. The first backs up over the superscript '2', the second
backs up over the '^' superscript operation; when entering the superscript
the baseline of the font and the current pen position is changed
immediately. So if that is not undone before staring to render further
characters, the pen position is not where you hoped it would be.
Found out that the fortron giza pgplot binding implemented PQQINF() (to
query version &cet) but the C-language binding didn't.
Whilst implementing cpgqinf() found that giza_query_device() and
_giza_int_to_device() contained a serious bug, with SIGSEGV looming or loss
of information.
Both quoted functions copy information into a user allocated string buffer.
Extract of relevant code with 'noise' deleted:
_giza_int_to_device (int numDevice, char *DeviceName)
{
case GIZA_DEVICE_NULL:
strncpy(DeviceName, "/null", sizeof(DeviceName)-1);
break;
....
The user allocated buffers are passed by pointer ("char* DeviceName") and
the 'sizeof(...)' operator returns the size of the _pointer_ not the size of
the buffer it's pointing at!
Depending on the platform "sizeof(char*) - 1" evaluates to 3 (32 bit) or 7
(64 bit), irrespective of how many characters the user really allocated.
This can lead to either:
- overestimating the amount of characters available in case the user
allocated less than 3 (or 7) bytes for their buffer. A strncpy() with
a source longer than 3 (or 7) would then overwrite memory that it
shouldn't, which in turn would lead to unpredictable behaviour at best
and program termination via SIGSEGV at worst.
- underestimating the amount characters available if the user allocated
more than 3 (or 7) characters for their target buffer. In this case,
the strncpy() with a source string > 3 (or 7) characters would
truncate the output - e.g. the file name of a device would easily
be > 7 characters long but at most 3 (or 7) of them would be copied.
This would lead to unnecessary loss of information.
In order to fix this the target buffer size must be explicitly passed to the
functions by the caller. Thus the prototype of giza_query_device() needed to
be altered and as a result the Fortran wrapper needed adapting to the new
signature and pass in the extra variable accordingly.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to my inadequate regression test scheme missed an off-by-one in
giza_parse_string()backspace handling; it worked for the tests I checked but I did not carefully enough test allpgdemo's. One line change.Added support for
cpgqinf()to query device information.PGQINF()was implemented in the FORTRAN pgplot library but not in the C-language binding.Whilst adding support for
cpgqinf()I found a critical misconception in twogiza_...functions used by this and fixed them. This required changes in the FORTRAN binding code too. See the commit message.