Make EINVAL handling more robust and revert binary

[cmorty]

As of #1073 this fixes things.

[alignan]

👍

[simonduq]

why // comments instead of /**/?

[simonduq]

system -> systems

[cmorty]

@simonduq: Thanks. Seems like today is a dyslexia-day. /me somtimes hates his brain.

[simonduq]

we've all been there ;)

[simonduq]

excellent, 👍 (but let's keep the pull open a bit longer before merging to give others a chance to review it)

[cmorty]

Sound like a good idea. ;) My PRs weren't very good lately. :(

[g-oikonomou]

1e359d2 from #1048 broke compilation on OS X :( Any chance we could look into this as well? Happy to test recommended changes if you don't have access to OS X

make serialdump
cc     serialdump.c   -o serialdump
serialdump.c:168:52: error: use of undeclared identifier 'O_DIRECT'
  fd = open(device, O_RDWR | O_NOCTTY | O_NDELAY | O_DIRECT | O_SYNC);
                                                   ^
serialdump.c:173:54: error: use of undeclared identifier 'O_DIRECT'
    fd = open(device, O_RDWR | O_NOCTTY | O_NDELAY | O_DIRECT);
                                                     ^

[simonduq]

shouldn't #ifdef O_SYNC be replaced with #ifdef O_DIRECT?

[cmorty]

@simonduq You're totally right. I should refrain from making PRs while writing my phd. It just makes me feel even more incompetent. ;)
@g-oikonomou Does this fix it for you?

[g-oikonomou]

This compiles now, yes.

[alignan]

Works for me, thanks for the work put into this! but why did you revert the binary instead of producing a new one?

[laurentderu]

I'm still wondering why we need O_DIRECT ? AFAIK, the effect of O_DIRECT is to bypass the kernel cache and use DMA if possible, but the bitrate is so low and anyway we are using O_SYNC to wait for the data to be written.

[cmorty]

@alignan I don't have a 32bit system at hand. I could compile using -m32, but again, testing would be a good idea.
@laurentderu You are probably right. I just didn't want to break things I'm not able to test.

[g-oikonomou]

I wonder whether there are historic reasons why we are shipping binaries here :)

[cmorty]

@g-oikonomou Other make-files depend on them. They would need to call make first. Though adding another target shouldn't be too complicated. Also it it quite convenient to download the binary without the need of compiling it.

[laurentderu]

+1 for the removal of the binary. Every once in a while, I forgot to recompile it on ARM platform and wonders why it does not work anymore :)

[g-oikonomou]

The counter-argument would be: ship it as part of instant contiki, perhaps somewhere inside the PATH. Perhaps even provide 3-4 pre-compiled versions somewhere e.g. in the wiki. Remove it from git and document how to build it for git users. This would have prevented the situation you are having to deal with now.

[cmorty]

What about shipping precompiled versions that get copied if there is no gcc available?

[g-oikonomou]

I am not 100% I understand what you had in mind, but I do find it unlikely for people to be using git without having gcc installed in some shape or form.

Just to be clear, I am not suggesting that this pull is not acceptable without removing the binary. I was just wondering aloud about why we were shipping it in the first place and whether we want to keep doing so

[cmorty]

@g-oikonomou I was fully aware that we were getting off-topic. ;) But to continue that and get more verbose about my idea:

• Adjust the rest of the system to build and use serialdump instead of serialdump-*
• Adjust the makefile to check whether there is a gcc. Otherwise copy the seiraldump-*
  For convenience one could add binaries for other systems, too. While hosting them somewhere is an option, too, not having to search them is a nice to have.

[oliverschmidt]

Hi,

Another aspect maybe worth keeping in mind here is that one needs to be in general very cautious about storing binaries in Git especially when they tend to change now and then as all versions of all binaries become part of the Git repo.

I learned the hard way myself that this way a Git repo can become quickly too large to handle - and that it is major pain in the neck to get rid of those artifacts afterwards.

Just my two cents,
Oliver

[g-oikonomou]

So the more I'm looking at this, the more I'm getting convinced that those (and perhaps other binaries too) need getting rid off.

• Adjust the rest of the system to build and use serialdump instead of serialdump-*

Probably a good idea. git grep serialdump suggests this won't be too big a pain.

By the way, there is a slightly different serialdump under the stm32 folder. "There can be only one" ultimately.

• Adjust the makefile to check whether there is a gcc. Otherwise copy the seiraldump-* For convenience one could add binaries for other systems, too.

Still not convinced 100% (nor do I understand 100%). Copy serialdump- from where to where? If you absolutely had to provide serialdump- without building it, then I would be strongly in favour of wgetting it instead of shipping it under version control.

And just to keep digressing off topic, the entire tools dir needs a little love. python23.dll? Seriously? 😄 Many of those things can (and perhaps should) be hosted as submodules under contiki-os (and I remember pretty well that you have suggested so in the past - quite passionately on occasion ;) )

[cmorty]

@g-oikonomou You convinced me. :) I still didn't manage to get Cooja out of Contiki, though. :)

[alignan]

I'm OK to merge as it is, then the thread comments can be addressed in a separate PR 👍

[cmorty]

@alignan @g-oikonomou : I can also rebase without e38bc3b. But currently I do not have the time to fix those make-files to get rid of the binary.

[alignan]

I'm OK either way, no strong opinion about the binary.

[g-oikonomou]

No need to get rid of the binary within this particular pull

[simonduq]

👍