On 08/03/19 09:28, John Morris wrote:
If I'd double-checked e207745, it would've been clear what was wrong:  L3 
should have stayed as ULL.  D'oh!

Mick, I'd like to revert your commit 730837e, address all problems, and submit 
a PR.  Of course e207745 caused the problem Dennis encountered, and I have a 
fix for it.  However, it doesn't explain why it reverts b55b544 (or more likely 
I'm missing something).  What's the problem there?
No specific problem, looked like that was the other bit of code which threw a warning reading the same data
so I just reverted it back as was.

Since I had reverted to the old values in one, I expected it to error looking for the new value in the other.

Was easier just to revert the associated commits, than find that whilst it built (albeit with warnings)
haltalk could not make any sense of the data passed.

No, I'm not looking forward to silencing warnings in gcc 8.  ;(

     John

________________________________________
From: schoone...@gmail.com <schoone...@gmail.com>
Sent: Thursday, March 7, 2019 5:35 PM
To: John Morris; machinekit@googlegroups.com
Subject: Re: [Machinekit] Mesa 6i25 & 7i76 - Operation not permitted with new 
Machinekit Version


On 07/03/19 07:19, John Morris wrote:
Ugh, sorry for introducing this problem, and thanks for everyone's help 
figuring it out.
One of those things John, if you don't do anything, you will never make
a mistake :)
I think it's important to fix compiler warnings.  When fixing these warnings, I 
found a couple of instances where the warnings actually pointed out real bugs.  
Warnings help me in my own development, and it's hard to see them in a haystack 
of other, unrelated warnings in the code (clean build log output was one of my 
motivations to fix the warnings in the first place).  I bet there are other 
good reasons for fixing them.  Am I alone on this one?
The fixes are good, allows for a much cleaner build.  Also picks up on
sloppy coding style etc. where real problems can lurk.
I'll fix this one properly so we can turn `-Werror` back on.  I'll also take a 
look at Buster builds down the road when I actually start using it, if others 
think fixing warnings is worthwhile; if not, I won't fight the current.
Thanks, it will work perfectly well in the interim, don't worry until
you have the time.

Every increment of gcc seems to bring stricter standards adherence and
hence more warnings.

This has been beneficial in the past, where for instance we realised
that a maths library appeared to have been written in C by a python
programmer,
because a whole group of conditional operations were indented but not
within parenthesis, meaning only the first line was conditional and
every other
line was executed whatever the outcome of the conditional test.
That situation appeared to have existed in linuxcnc also, for many years.

I will wait for you to discover the wonders of gcc 8.xx for yourself and
see if you can live with them :D

      John

________________________________________
From: machinekit@googlegroups.com <machinekit@googlegroups.com> on behalf of 
schoone...@gmail.com <schoone...@gmail.com>
Sent: Wednesday, March 6, 2019 10:32 PM
To: machinekit@googlegroups.com
Subject: Re: [Machinekit] Mesa 6i25 & 7i76 - Operation not permitted with new 
Machinekit Version

Yes, I read something very similar on stackoverflow Charles.
It is probably the way to go, certainly to test out.

As I didn't have the ability right now to test on real hardware,
I just reverted to be safe.

The warnings issue on Jessie and Stretch is as nothing to what is to come
in Buster with gcc 8.xx

There are some pretty intractable warnings due to the coding style in our
legacy code, where buffers are allocated to a defined length
and then a subsequent strncpy() or whatever is fixed to that same
defined length.

The later version of gcc will both warn of potential buffer overflow and of
potential data truncation, even if neither are possible in that instance.

Removing those, if ever deemed worth spending time on, will mean re-writing
a lot of code, possibly with some quite clunky stuff.


On 06/03/19 14:08, Charles Steinkuehler wrote:
On 3/6/2019 5:30 AM, schoone...@gmail.com wrote:
It essentially comes down to 32 bit and 64 bit differences in data type size.
If you then specify a format size in a printf operation, it will always generate
a warning under one architecture or another.
Assigning with a (void*) cast, will do so too, hence the making of L1 and L2
void * probably
I don't have time to test this in Machinekit, but I've solved these
sort of issues using the intptr_t data type (which changes size based
on the architecture), so something like:

     unsigned intptr_t L1, L2;
     unsigned long L3;

Another option (since intptr_t isn't guaranteed to be available and
I'm not 100% the pointer casts required for fscanf wouldn't throw a
warning) is to make a union and overlap a pointer with an integer
type, eg:

     union foo {
       unsigned long long foo_long;
       void *foo_ptr;
     }

...then use the appropriate type where needed.

--
website: http://www.machinekit.io blog: http://blog.machinekit.io github: 
https://github.com/machinekit
---
You received this message because you are subscribed to the Google Groups 
"Machinekit" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to machinekit+unsubscr...@googlegroups.com.
Visit this group at https://groups.google.com/group/machinekit.
For more options, visit https://groups.google.com/d/optout.

--
website: http://www.machinekit.io blog: http://blog.machinekit.io github: 
https://github.com/machinekit
--- You received this message because you are subscribed to the Google Groups "Machinekit" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to machinekit+unsubscr...@googlegroups.com.
Visit this group at https://groups.google.com/group/machinekit.
For more options, visit https://groups.google.com/d/optout.

Reply via email to