I just tried that on one file and it compiles without warnings but looks
like it needs to be added to the Submakefiles in each of the ../src/
subdirectories to link the library. I'm not sure how to do that..
libbsd-dev was already installed on my Chromebook so it does not change the
dependencies.
Rod Webster
*1300 896 832*
+61 435 765 611
Vehicle Modifications Network
www.vehiclemods.net.au
On Sat, 12 Feb 2022 at 16:09, Andy Howell <a...@gamubaru.com> wrote:
>
> On 2/11/22 20:24, Rod Webster wrote:
> > I had a quick look at this. Some errors stem from using int instead of
> > size_t when defining the variable receiving the return value.
> > eg line 200 of inifile,cc should be defined as :
> > size_t newLinePos; /* position
> of
> > newline to strip */
> >
> > But where the string is truncated it seems the strlen has already been
> > calculated and we are only copying the required characters into a fixed
> > length buffer so we clearly do know what we are doing. Disabling the
> > warning can be done with:
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Wstringop-truncation"
> > //
> > // some code that generates warnings
> > //
> > #pragma GCC diagnostic pop
> >
> > eg. by placing the first 2 lines at the top of inivar.c and the last line
> > at the end of the file to restore the default error handling, warnings
> are
> > no longer displayed.
> > To my mind, that is the preferred solution as adding obfuscated code to
> > avoid a warning is worse than the error.
> > The main thing is we ensure all warnings are reviewed and where the code
> > can't be corrected without obfuscation, the warning is suppressed for
> that
> > section of code.
>
> Rod,
>
> I thought about pragmas. That is cleaner, though it does not catch the
> corner case where the source string is longer than the n number of
> bytes. No null is written in that case.
>
> The other option would be to use strlcpy, but that requires libbsd.
> Classicladder, halcmd, halmeter, halscope and rs274 link to that, so its
> not a new dependency. That makes sure the string is always null
> terminated, even if truncated.
>
> Andy
>
> > Rod Webster
> > *1300 896 832*
> > +61 435 765 611
> > Vehicle Modifications Network
> > www.vehiclemods.net.au
> >
> >
> > On Sat, 12 Feb 2022 at 07:39, Andy Howell <a...@gamubaru.com> wrote:
> >
> >> On 2/11/22 09:10, Jared McLaughlin wrote:
> >>> Andy,
> >>>
> >>> I'm a reasonable C/C++ programmer as well. I'd be willing to be a
> hunting
> >>> partner if you want to go bug hunting together. Did you build on a
> fresh
> >> OS
> >>> install?
> >>>
> >>> Jared
> >> Jared,
> >>
> >> No, I'm just building this on my Ubuntu 21.10 laptop. I'm using gcc
> >> 11.2.0 to compile it. That may spit out more warnings that what Buster
> >> uses. I don't know.
> >>
> >> I'd welcome help on it. I'm open to better ways to handle errors like
> this:
> >>
> >> Compiling hal/user_comps/mb2hal/mb2hal_init.c
> >> In file included from /usr/include/string.h:519,
> >> from hal/user_comps/mb2hal/mb2hal.h:4,
> >> from hal/user_comps/mb2hal/mb2hal_init.c:1:
> >> In function ‘strncpy’,
> >> inlined from ‘init_mb_links’ at
> >> hal/user_comps/mb2hal/mb2hal_init.c:717:17:
> >> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:95:10: warning:
> >> ‘__builtin_strncpy’ output may be truncated copying 16 bytes from a
> >> string of length 16 [-Wstringop-truncation]
> >> 95 | return __builtin___strncpy_chk (__dest, __src, __len,
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> 96 | __glibc_objsize (__dest));
> >> | ~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> The code in question, with my 'fix' is:
> >>
> >> else { //tcp
> >> strncpy(this_mb_link->lp_tcp_ip,
> >> this_mb_tx->cfg_tcp_ip, sizeof(this_mb_tx->cfg_tcp_ip)-1);
> >> + this_mb_link->lp_tcp_ip[sizeof(this_mb_tx->cfg_tcp_ip)-1] ='\0';
> >> this_mb_link->lp_tcp_port=this_mb_tx->cfg_tcp_port;
> >>
> >> Terminating the string makes the compiler happy, even though is only
> >> required when the source string is exactly n bytes long.
> >>
> >> That solution works. It does not cope with possible truncation of the
> >> source string. I don't know the code enough to know which areas are time
> >> sensitive, so I wasn't thinking of more CPU intensive fixes like
> >> computing all the lengths and error checking.
> >>
> >> My main aim is to reduce the warnings, so that when new warnings pop up,
> >> they get attention and dwelt with quickly. Even if its only tell the
> >> compiler "don't worry, we know what we're doing."
> >>
> >> Andy
> >>
> >>
> >>> On Thu, Feb 10, 2022, 4:50 PM Andy Howell <a...@gamubaru.com> wrote:
> >>>
> >>>> On 2/10/22 06:10, Steffen Möller wrote:
> >>>>> On 10.02.22 07:28, Phill Carter wrote:
> >>>>>>> On 10 Feb 2022, at 4:14 pm, Andy Howell <a...@gamubaru.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 2/9/22 14:46, andy pugh wrote:
> >>>>>>>> On Wed, 9 Feb 2022 at 20:21, Andy Howell <a...@gamubaru.com>
> wrote:
> >>>>>>>>
> >>>>>>>>> My background is in c/c++ development under UNIX/Linux. I know
> bit
> >> of
> >>>>>>>>> python. I don't have the experience to work on the low level
> >>>>>>>>> internals.
> >>>>>>>> How about this one?
> >>>>>>>> https://github.com/LinuxCNC/linuxcnc/issues/1438
> >>>>>>>>
> >>>>>>>> A text search on the pin name would find the right part of the
> code,
> >>>>>>>> and would let you find where the associated internal variable is
> >>>>>>>> updated.
> >>>>>>>>
> >>>>>>>> (This is internals, but with code to copy from)
> >>>>>>> Thanks. That sounds like a good problem to start with. I will have
> a
> >>>>>>> look at the code and comment under the issue.
> >>>>>> I do have a PR ready to submit but I am happy to hold off.
> >>>>> That sounds so nice and so wrong at the same time. I suggest to
> submit
> >>>>> the PR and you have already found a reviewer.
> >>>>>
> >>>>> @Andy, I am currently on a mission to render the C/C++ sources
> >>>>> cppcheck-clean. You see a few PRs on this already - and one issue
> that
> >> I
> >>>>> could not solve with a quick patch that I think is a bug. I
> personally
> >>>>> use this to get acquainted with the code base a bit more - have
> started
> >>>>> with src/hal/* and src/emc/* and have just about completed that, I
> >>>>> think. But there is more.
> >>>>>
> >>>>> Many thanks and greetings
> >>>>> Steffen
> >>>>>
> >>>> Steffen,
> >>>>
> >>>> I could certainly work on that. I just compiled LinuxCNC for the first
> >>>> time in a very long time. There were lots of compiler warnings,
> >>>> particularly around strncpy and snprintf. I think those would be worth
> >>>> looking into. It would be nice to fix the code to remove the warnings,
> >>>> or turn warnings off where needed when they are clearly a false
> >>>> positive. For example,
> >>>>
> >>>> hal/user_comps/mb2hal/mb2hal_init.c:187:55: warning: ‘%02d’ directive
> >>>> output may be truncated writing between 2 and 10 bytes into a region
> of
> >>>> size 7 [-Wformat-truncation=]
> >>>> 187 | snprintf(section, sizeof(section)-1,
> "TRANSACTION_%02d",
> >>>> mb_tx_num);
> >>>> | ^~~~
> >>>> hal/user_comps/mb2hal/mb2hal_init.c:187:42: note: directive argument
> in
> >>>> the range [0, 2147483647]
> >>>> 187 | snprintf(section, sizeof(section)-1,
> "TRANSACTION_%02d",
> >>>> mb_tx_num);
> >>>> | ^~~~~~~~~~~~~~~~~~
> >>>>
> >>>> The local 'section' buffer there is 20 bytes. Although it might never
> be
> >>>> an issue in practice, increasing the buffer size will make sure it
> never
> >>>> is and remove the warning. Maybe we can get to a point where we can
> >>>> treat warnings as errors.
> >>>>
> >>>> Does that sound worth pursuing?
> >>>>
> >>>>> _______________________________________________
> >>>>> Emc-developers mailing list
> >>>>> Emc-developers@lists.sourceforge.net
> >>>>> https://lists.sourceforge.net/lists/listinfo/emc-developers
> >>>> _______________________________________________
> >>>> Emc-developers mailing list
> >>>> Emc-developers@lists.sourceforge.net
> >>>> https://lists.sourceforge.net/lists/listinfo/emc-developers
> >>>>
> >>> _______________________________________________
> >>> Emc-developers mailing list
> >>> Emc-developers@lists.sourceforge.net
> >>> https://lists.sourceforge.net/lists/listinfo/emc-developers
> >>
> >> _______________________________________________
> >> Emc-developers mailing list
> >> Emc-developers@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/emc-developers
> >>
> > _______________________________________________
> > Emc-developers mailing list
> > Emc-developers@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/emc-developers
>
>
> _______________________________________________
> Emc-developers mailing list
> Emc-developers@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/emc-developers
>
_______________________________________________
Emc-developers mailing list
Emc-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/emc-developers
