You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/03/12 02:01:57 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
yamt opened a new pull request #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548
The remaining errors:
Operator/assignment must be preceded with whitespace
I didn't fix them because they are in assembly code, which nxstyle doesn't understand.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598864132
Then we could check for "#ifdef __ASSEMBLY__" inside header files and ignore file sections guarded by it. That would fix already many if not most of the issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus edited a comment on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus edited a comment on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598864132
Then we could check for
```
#ifdef __ASSEMBLY__
```
inside header files and ignore file sections guarded by it. That would fix already many if not most of the issues.
Edit: I had a closer look at the assembly header files and I think evaluating
```
#ifdef __ASSEMBLY__
```
and then skipping some checks would be enough. I'd like to keep at least the preprocessor lines and comment checks in place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-597988609
ok. then we need either
* reviewers inspect errors manually
* or, find an automatic way to detect assembly and skip them
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-597976013
>
>
> my suggestion is to stop using *.h for assemby. eg. use *.inc instead.
> note: this might be a trivial change because some files are included by both of C and asm.
This is not a NuttX convention. .inc is not used by any files in NuttX. It is a converntion of certain vendors of support libraries for MCUs. We cannot change it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598850647
the header section of the file nevertheless says:
arch/xtensa/src/common/xtensa_cpuint.S
Since ".s" is a common used extension for assembly files, why don't we use it?
I know,
´´´
#include "xtensa_abi.s"
´´´
would look a little weird but it does not violate any standard (afaik).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus edited a comment on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus edited a comment on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598864132
Then we could check for
```
#ifdef __ASSEMBLY__
```
inside header files and ignore file sections guarded by it. That would fix already many if not most of the issues.
Edit: I had a closer look at the assembly files and I think evaluating
```
#ifdef __ASSEMBLY__
```
and then skipping some checks would be enough. I'd like to keep at least the preprocessor lines and comment checks in place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus edited a comment on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus edited a comment on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598850647
the header section of the file nevertheless says:
arch/xtensa/src/common/xtensa_cpuint.S
Since ".s" is a common used extension for assembly files, why don't we use it?
I know,
```
#include "xtensa_abi.s"
```
would look a little weird but it does not violate any standard (afaik).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo merged pull request #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598987771
I don't see any strong reason to hold this up. It is unfortunate that the other tests were blocked but the nxstyle complaints are bogus. Those are errors for _asm code which nxstyle does not handler.
This other discussion, while interesting, don't see to have anything to do with the disposition of the PR. That discussion should probably be moved to an open issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus edited a comment on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus edited a comment on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598864132
Then we could check for
```
#ifdef __ASSEMBLY__
```
inside header files and ignore file sections guarded by it. That would fix already many if not most of the issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] johannes-nivus edited a comment on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
johannes-nivus edited a comment on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598864132
Then we could check for
```
#ifdef __ASSEMBLY__
```
inside header files and ignore file sections guarded by it. That would fix already many if not most of the issues.
Edit: I had a closer look at the assembly files and I think by evaluating
```
#ifdef __ASSEMBLY__
```
and then skipping some checks would be enough. I'd like to keep at least the preprocessor lines and comment checks in place.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
yamt commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-597972832
my suggestion is to stop using *.h for assemby. eg. use *.inc instead.
note: this might be a trivial change because some files are included by both of C and asm.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #548:
arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #548: arch/xtensa/src/common/xtensa_abi.h: nxstyle fixes
URL: https://github.com/apache/incubator-nuttx/pull/548#issuecomment-598852107
Lower case .s files are assembly files that do not go to the pre-processor. Upper case .S files need to be pre-processes. .s is already used by GCC. http://labor-liber.org/en/gnu-linux/development/index.php?diapo=extensions
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services