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 2021/05/04 18:11:16 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #3657: ESP32&ESP32C3/hardware: Include files of the same level by their names only
Ouss4 opened a new pull request #3657:
URL: https://github.com/apache/incubator-nuttx/pull/3657
## Summary
ESP32&ESP32C3: Include files from hardware folder by their names only.
## Impact
ESP32 and ESP32-C3 only.
## Testing
ESP32 & ESP32-C3 boards.
--
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
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3657: ESP32&ESP32C3/hardware: Include files of the same level by their names only
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3657:
URL: https://github.com/apache/incubator-nuttx/pull/3657#discussion_r626045590
##########
File path: arch/risc-v/src/esp32c3/hardware/esp32c3_gpio.h
##########
@@ -25,7 +25,7 @@
* Included Files
****************************************************************************/
-#include "hardware/esp32c3_soc.h"
+#include "esp32c3_soc.h"
Review comment:
I believe this case here is a bit different.
The intention is to rely on the GCC behavior of primarily looking for the included file on the same directory of the current file, as documented here: https://gcc.gnu.org/onlinedocs/cpp/Search-Path.html
So, as the search path will not need to be considered in this process, we might be safe.
Furthermore, the header files from the `hardware` folder usually do not include files from other include paths except from the `hardware` folder itself. Although this might not be a golden rule, it also lessens the chance of occurring unpleasant surprises.
--
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
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #3657: ESP32&ESP32C3/hardware: Include files of the same level by their names only
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #3657:
URL: https://github.com/apache/incubator-nuttx/pull/3657#discussion_r626016997
##########
File path: arch/risc-v/src/esp32c3/hardware/esp32c3_gpio.h
##########
@@ -25,7 +25,7 @@
* Included Files
****************************************************************************/
-#include "hardware/esp32c3_soc.h"
+#include "esp32c3_soc.h"
Review comment:
Careful. In some cases, there is are header files of the same name at arch/xxx/src/xxx and at arch/xxx/src/xxx/hardware/. In that case, the hardware/ prefix can be required if your include patch has ".", arch/xxx/src/chip and arch/xxx/src/xxx/hardware on it. In that case, the order will depend on the context and the order of the include paths on on the command line.
This sometimes catches you if "." is not where you think it might be. I have had to add the hardware/ prefix to other architectures in the past to avoid including the wrong file. Also, I suspect that there might be issues in certain cases if arch/xxx/src/xxx/hardware/header.h includes arch/xxx/src/chip/header.h. I have experienced that even though arch/xxx/src/chip appeared on the include search path first. I never really understood, but adding hardware/ eliminated the bad inclusion.
A safer solution would be to remove arch/xxx/src/xxx/hardware from the include search path and always preface the inclusion with hardware/.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #3657: ESP32&ESP32C3/hardware: Include files of the same level by their names only
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3657:
URL: https://github.com/apache/incubator-nuttx/pull/3657
--
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
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #3657: ESP32&ESP32C3/hardware: Include files of the same level by their names only
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #3657:
URL: https://github.com/apache/incubator-nuttx/pull/3657#discussion_r626393716
##########
File path: arch/risc-v/src/esp32c3/hardware/esp32c3_gpio.h
##########
@@ -25,7 +25,7 @@
* Included Files
****************************************************************************/
-#include "hardware/esp32c3_soc.h"
+#include "esp32c3_soc.h"
Review comment:
> A safer solution would be to remove arch/xxx/src/xxx/hardware from the include search path and always preface the inclusion with hardware/.
Is arch/xxx/src/xxx/hardware in the include search path? I can't see it.
As Gustavo mentioned this relies on the fact that `" "` includes will look first in the current directory. I only changed files from that same directory.
--
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