search

thesofproject / rimage

DSP firmware image creation and signing tool
Other
7 stars 62 forks source link

Cppcheck fixes but its for rimage #46

Closed cujomalainey closed 3 years ago

cujomalainey commented 3 years ago

@lgirdwood do you know why the workflows are not being recognized? Also @marc-hb I have a commit to update this to actions and off travis

marc-hb commented 3 years ago

Please try to copy/paste one of these two files into https://github.com/thesofproject/rimage/actions/new

For the main repo I think I remember nothing happened until I started using the web interface.

cujomalainey commented 3 years ago

@marc-hb i think i figured it out, likely wont trigger new files when being pulled from an external fork for security reasons, since it ran fine in my repo

cujomalainey commented 3 years ago

nevermind, added and deleted a commit and now it all works. i blame cosmic radiation

marc-hb commented 3 years ago

i blame cosmic radiation

I did not complete it but I did initiate the addition of a github action from the web interface... blame me?

cujomalainey commented 3 years ago

i blame cosmic radiation

I did not complete it but I did initiate the addition of a github action from the web interface... blame me?

Seems to be a common issue https://github.community/t/workflow-dispatch-workflow-not-showing-in-actions-tab/130088/25

cujomalainey commented 3 years ago

Sorry for the delay, got caught up in internal stuff

cujomalainey commented 3 years ago

Travis is failing because checkpatch is mad that im using memcpy vs memcpy_s. Using the latter will require more surgery since size is not passed into the function. @lgirdwood what are your thoughts here

cujomalainey commented 3 years ago

FWIW i tried to update to memcpy_s but the docker container we use right now doesn't have the definition so it breaks it

cujomalainey commented 3 years ago

Rimage commit 740b5f54c94cffc7 tested in https://github.com/thesofproject/sof/pull/4205 and everything is green (I hate to admit that submodules are very well supported by github)

FWIW i tried to update to memcpy_s but the docker container we use right now doesn't have the definition so it breaks it

Does any Linux distro have memcpy_s()?

Github does do an amazing job of handling the evil that is submodules.

I'm not sure what is going on with the container. It's Ubuntu 20.04. memcpy_s has been part of gcc since ~4.8 log shows 9.x. I even checked /usr/include/string.h and the signature was not there.

marc-hb commented 3 years ago

Do you have memcpy_s on your Linux system and if yes then what distribution is it?

memcpy_s has been part of gcc since ~4.8

I think memcpy and similar functions are part of libc, not of any compiler. In fact we recently had an issue where the gcc -O optimizer was auto-generating calls to these functions that were not even explicit in the source code and that was causing the linker to fail: https://github.com/thesofproject/sof/issues/3880#issuecomment-796537478

In the past the glibc maintainer(s) have expressed very negative opinions about these extensions submitted by Microsoft.

dpkg -S /usr/include/string.h 
libc6-dev:amd64: /usr/include/string.h
cujomalainey commented 3 years ago

Do you have memcpy_s on your Linux system and if yes then what distribution is it?

dpkg -S /usr/include/string.h 
libc6-dev:i386, libc6-dev:amd64: /usr/include/string.h

Only place I can see it is either in code that xcc compiles (aka SOF) or in one very obscure place I would not expect to be using any headers from. This is a fork of debian. Compiling a test program in the base OS also fails. So we can't assume its there it seems.

lgirdwood commented 3 years ago

Travis checkpatch failure, build is good.