Hi all (Giles in particular),
I wanted to pass on a couple of issues I ran in to while working on
getting version 2-4-10 of the Power PMAC driver working on a machine
running Debian Stretch.
The first issue I ran in to is rather trivial:
This call in pmacCSAxis.cpp:
sprintf(acc_buff, pC_->getCSAccTimeCmd(fabs(max_velocity /
acceleration) * 1000.0).c_str());
and this call in interactive.cpp:
printf(buff);
The new system have vers 6.3.0 of g++, and the list of options includes
-Werror=format-security. This means that the use of non-constant (or
non literal?) strings for what is normally the "format" string is
treated as an error. This is a reasonable safeguard, since if there was
an error in the generation of the "format" string that resulted in a "%"
char being in it, that could cause all kinds of trouble.
In any case, the solution is simple enough: Simply add an actual format
string argument: "%s". Although in the first case, it seems like it
would make sense to just replace sprintf() with a copy operation.
The next issue I found appears to be a difference in how the compiler
handles this statement in pmacCommandStore::buildCommandString():
sprintf(commandString[qtyCmdStrings], "%s %s",
commandString[qtyCmdStrings], key.c_str());
On my older development machine running Jessie (with g++ vers 4.9.2),
this has the intended effect, which is to add the value of key.c_str()
to the end of the current contents of commandString[qtyCmdStrings].
On my newer machine running Stretch (with g++ vers 6.3.0), the result is
that current contents of commandString[qtyCmdStrings] is replaced with a
space followed by the value of key.c_str(). This call occurs in two
places: Once before a loop (to initialize the commandString[] element),
so the result is what is wanted (even if mostly by accident in this
case). But for the call inside the loop, the result is that the
commandString[...] value contains only a space followed by the last
value key.c_str() had (rather than the list of commands it was intended
to contain).
I was tempted to fix this by doing something like this:
int curLen = strlen(commandString[qtyCmdStrings]);
sprintf(commandString[qtyCmdStrings]+curLen, "%s %s",
commandString[qtyCmdStrings], key.c_str());
This would PROBABLY be safe, but since I don't know exactly what the
underlying logic of sprintf() is doing now (or perhaps some new compiler
optimization?), it seemed safer to NOT use the same array in both the
target and the source arguments. So instead I changed it to copy the
current value to a temp string each time and use that in the call:
char curStr[1024];
...
strcpy(curStr, commandString[qtyCmdStrings]);
sprintf(commandString[qtyCmdStrings], "%s %s", curStr, key.c_str());
There are of course lots of other ways this could be fixed (and
additional safeguards added), but this is sufficient to verify what the
problem is with the existing code when using the newer compiler and
libraries.
--------------------------
Mark Davis
NSCL/FRIB
Control Systems Software Engineer
[email protected]
___________________________________________________________________________________________________
- Replies:
- Re: pmac driver on Debian Stretch J. Lewis Muir via Tech-talk
- Navigate by Date:
- Prev:
Pointer corruption on Windows Koennecke Mark (PSI) via Tech-talk
- Next:
RE: [motorPIGCS2] can't turn on servo state of PI E-712 Allan Serra Braga Bugyi via Tech-talk
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
<2019>
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
RE: Pointer corruption on Windows Freddie Akeroyd - UKRI STFC via Tech-talk
- Next:
Re: pmac driver on Debian Stretch J. Lewis Muir via Tech-talk
- Index:
1994
1995
1996
1997
1998
1999
2000
2001
2002
2003
2004
2005
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
<2019>
2020
2021
2022
2023
2024
|