On 2/11/22 21:29, Jared McLaughlin 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,

Definitely don't want to introduce silent bugs. Being new here, that would be a terrible way to start! That is why I think it would be good for us to tag-team. Get two sets of eyes on it before we ask anyone to review it.

I didn't have any problems building once I got the dependencies. dpkg-checkbuilddeps found most of them, but there were a few it missed. I should have written those down.

Andy

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

Reply via email to