Linuxcnc's buildbot automates the build process and hopefully it won't be
long before linuxcnc is included in Debian 12 repos. The docs Building
linuxcnc section covers how to resolve build dependencies fairly well (in
a cryptic fashion).
You have to look at the context of the code before suppressing anything.
Don't forget that linuxcnc is a very old code base so some things were not
warnings when the code was written.
When you define a 255 char buffer to read a line into and then
subsequently read a line of code that is only say 79 bytes long into this
buffer, then copy just those 79 bytes elsewhere, a
stringop-truncation warning will be triggered every time. To resolve the
warning would require copying all 255 characters and incur an
unnecessary overhead. In this case, I think suppressing the error is
warranted, particularly where we are dealing with tried and tested code as
its well written, easy to understand and efficient.
I think that this one is a possible gotcha
libnml/cms/cms_cfg.cc: In function ‘int load_nml_config_file(const char*)’:
libnml/cms/cms_cfg.cc:113:12: warning: ‘char* strncpy(char*, const char*,
size_t)’ specified bound 80 equals destination size [-Wstringop-truncation]
113 | strncpy(info->file_name, file, 80);
| ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
the procedure receives a pointer to *file, so there is no knowledge of the
string length. Garbage could result!
I think also this one is a concern
In function ‘char* get_buffer_line(const char*, const char*)’:
cc1plus: warning: function may return address of local variable
[-Wreturn-local-addr]
libnml/cms/cms_cfg.cc:308:10: note: declared here
308 | char linebuf[LINELEN]; /* Temporary buffer for line from
| ^~~~~~~
I think the linebuf variable should be a static variable because a pointer
to this memory location is returned and being defined locally, it is
disposed of on exit.
I'm not one of the developers and just looked at the errors displayed in
order after typing make from the first 4-5 files. There is certainly scope
to improve our code by tackling this.
I hope Andy makes a difference.
Rod Webster
*1300 896 832*
+61 435 765 611
Vehicle Modifications Network
www.vehiclemods.net.au
On Sat, 12 Feb 2022 at 13:30, Jared McLaughlin <jared.p.mclaugh...@gmail.com>
wrote:
> Andy,
>
> I'm working on compiling in a clean install on a VM of Ubuntu 20.04. I'm
> going to use clang just to check compiler coverage.
>
> I'm noticing a bunch of build deps - which is not unexpected for a system
> of this size. However, it makes me wonder if something similar to conan
> would make sense to automate the build more.
>
> Or is there something in debian, since we know which packages we need, that
> will auto-install them?
>
> I agree that getting a 'clean compile' with no warnings is a noble goal.
> That said, I am remiss to make a quick fix without really knowing the
> ramifications. I'm even less happy about silencing warnings. In either
> case, we are silencing a legitimate warning system and possibly introducing
> silent bugs. Silent bugs are worse than issues we are aware of. I'll dig in
> to it more once I actually get a build.
>
> Jared
>
> On Fri, Feb 11, 2022 at 4:41 PM 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
