You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "danielappiagyei-bc (via GitHub)" <gi...@apache.org> on 2023/09/10 23:48:41 UTC
[GitHub] [nuttx] danielappiagyei-bc opened a new pull request, #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
danielappiagyei-bc opened a new pull request, #10565:
URL: https://github.com/apache/nuttx/pull/10565
## Summary
Rename the struct fields `public` and the function parameter `this` in `streams.h` and `rtti.h` so C++ compilers won't fail to compile them (these are [reserved keywords](https://en.cppreference.com/w/cpp/keyword) in C++). This fixes the issue called out by https://github.com/apache/nuttx/issues/10556
## Impact
`lib/libc/stream`, `segger/rtt.h`
## Testing
- Successfully compiled `nsh` app for IMXRT after I:
- modified it to include [segger/rtt.h](https://github.com/apache/nuttx/blob/master/include/nuttx/segger/rtt.h)
- set the following [segger Kconfigs](https://github.com/apache/nuttx/blob/master/drivers/segger/Kconfig): `STREAM_RTT`, `SEGGER_RTT`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1722227299
> @mu578 @xiaoxiang781216 Does nuttx have any automatic reformatting tool, like clang-format or something?
you can try:
https://github.com/apache/nuttx/blob/master/tools/uncrustify.cfg
https://github.com/apache/nuttx/blob/master/tools/detab.c
https://github.com/apache/nuttx/blob/master/tools/rmcr.c
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322313781
##########
arch/arm64/src/common/arm64_initialize.c:
##########
@@ -145,7 +145,7 @@ static void up_color_intstack(void)
****************************************************************************/
#ifdef CONFIG_ARCH_FPU
-int arm64_panic_disable_fpu(struct notifier_block *this,
+int arm64_panic_disable_fpu(struct notifier_block *panicblock,
Review Comment:
sure
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322313561
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -70,18 +71,19 @@ static void stdsostream_putc(FAR struct lib_sostream_s *self, int ch)
static int stdsostream_puts(FAR struct lib_sostream_s *self,
FAR const void *buffer, int len)
{
- FAR struct lib_stdsostream_s *stdsostream = (FAR struct lib_stdsostream_s *)self;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
nxstyle complained that the line was too long so I shortened the names to be under 80 characters
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1716048700
> @danielappiagyei-bc please squash your change into one patch.
@xiaoxiang781216 done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1714905466
> @danielappiagyei-bc please squash your change
squash just the last 2 commits, correct?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713131812
I've fixed a few missing updates and also created a PR in the nuttx-apps repo (https://github.com/apache/nuttx-apps/pull/2047) for the files there that need to be changed. I'm not sure what the workflow is like when changes need to be applied to both repos at the same time for either PR to pass
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] TimJTi commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "TimJTi (via GitHub)" <gi...@apache.org>.
TimJTi commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1727926390
> > @danielappiagyei-bc @xiaoxiang781216 looks like the apps directory was not included when this was done. Have found 4 references to stream.public that now cause a build fail.
> > Perhaps the files in the app folder can be "fixed" too? I will raise a new issue for this in the nuttx-apps repo.
>
> @TimJTi I apologize, I thought I had fixed everything in the apps repo. Yes, create a new issue please. Is CI failing too or just a specific local build?
At this stage a local build is failing - not just because of this but because of some other recent NuttX PR that means libapps.a can't be found or isn't being created!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1327912952
##########
libs/libc/stream/lib_lzfcompress.c:
##########
@@ -62,11 +62,11 @@ static int lzfoutstream_flush(FAR struct lib_outstream_s *this)
* Name: lzfoutstream_puts
****************************************************************************/
-static int lzfoutstream_puts(FAR struct lib_outstream_s *this,
+static int lzfoutstream_puts(FAR struct lib_outstream_s *self,
FAR const void *buf, int len)
{
FAR struct lib_lzfoutstream_s *stream =
- (FAR struct lib_lzfoutstream_s *)this;
+ (FAR struct lib_lzfoutstream_s *)self;
Review Comment:
need change line 86
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1722088215
@mu578 @xiaoxiang781216
Does nuttx have any automatic reformatting tool, like clang-format or something?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1727920348
> @danielappiagyei-bc @xiaoxiang781216 looks like the apps directory was not included when this was done. Have found 4 references to stream.public that now cause a build fail.
>
> Perhaps the files in the app folder can be "fixed" too? I will raise a new issue for this in the nuttx-apps repo.
@TimJTi
I apologize, I thought I had fixed everything in the apps repo. Yes, create a new issue please. Is CI failing too or just a specific local build?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322327107
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -70,18 +71,19 @@ static void stdsostream_putc(FAR struct lib_sostream_s *self, int ch)
static int stdsostream_puts(FAR struct lib_sostream_s *self,
FAR const void *buffer, int len)
{
- FAR struct lib_stdsostream_s *stdsostream = (FAR struct lib_stdsostream_s *)self;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
I'm just now realizing that it isn't required for the `(` to be at the same indent level as the `=` so I could have kept the same name and pushed the second line to the left more:
```
FAR struct lib_stdsostream_s *stdsostream =
(FAR struct lib_stdsostream_s *)self;
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713828734
> > @danielappiagyei-bc but the change from 'this' to 'self' lose?
>
> I only renamed the 'this' function argument to 'self' [here](https://github.com/apache/nuttx/pull/10565/files#diff-2dd8bd975a0b4fe946a837961405b253a447eb9822e8e9a4b43359aeea2b0188L73) in `include/nuttx/streams.h` since that's the only `.h` file I found it in. It's still used in several `.c` files such as https://github.com/apache/nuttx/blob/master/drivers/segger/stream_rtt.c#L87 but I left those alone since only the `.h` files need to be compatible with C++ compilers when you `#include` them. However, I can rename these remaining usages as well if you would like
it's better to unify the naming and avoid the user copy public/this to other places.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1714904733
@danielappiagyei-bc please squash your change
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713097440
> > looks like I didn't properly rebase and squash into one commit. Will fix
>
> Yes, please squash you patchset:
>
> ```
> git rebase --interactive HEAD~3
> git push -f origin
> ```
Sorry for the delay, I've rebased and squash now. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] TimJTi commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "TimJTi (via GitHub)" <gi...@apache.org>.
TimJTi commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1727915336
@danielappiagyei-bc @xiaoxiang781216
looks like the apps directory was not included when this was done. Have found 4 references to stream.public that now cause a build fail.
Perhaps the files in the app folder can be "fixed" too? I will raise a new issue for this in the nuttx-apps repo.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323311650
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
let's unify all ostream/istream/stdoutstream/stdistream to stream?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1327914351
##########
libs/libc/stream/lib_lzfcompress.c:
##########
@@ -62,11 +62,11 @@ static int lzfoutstream_flush(FAR struct lib_outstream_s *this)
* Name: lzfoutstream_puts
****************************************************************************/
-static int lzfoutstream_puts(FAR struct lib_outstream_s *this,
+static int lzfoutstream_puts(FAR struct lib_outstream_s *self,
FAR const void *buf, int len)
{
FAR struct lib_lzfoutstream_s *stream =
- (FAR struct lib_lzfoutstream_s *)this;
+ (FAR struct lib_lzfoutstream_s *)self;
Review Comment:
done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1722156819
I've now rebased into a single commit, let me know if there's any other changes required before merge please @xiaoxiang781216
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1714253140
Yeah thats true. It's an easy change on my end, I'll have the changes in maybe 9 hours when im free. Thanks!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713053350
> looks like I didn't properly rebase and squash into one commit. Will fix
Yes, please squash you patchset:
```
git rebase --interactive HEAD~3
git push -f origin
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] TimJTi commented on pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "TimJTi (via GitHub)" <gi...@apache.org>.
TimJTi commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1727930746
See apps [PR2078](https://github.com/apache/nuttx-apps/issues/2078)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713812028
> @danielappiagyei-bc but the change from 'this' to 'self' lose?
I only renamed the 'this' function argument to 'self' [here](https://github.com/apache/nuttx/pull/10565/files#diff-2dd8bd975a0b4fe946a837961405b253a447eb9822e8e9a4b43359aeea2b0188L73) in `include/nuttx/streams.h` since that's the only `.h` file I found it in. It's still used in several `.c` files such as https://github.com/apache/nuttx/blob/master/drivers/segger/stream_rtt.c#L87 but I left those alone since only the `.h` files need to be compatible with C++ compilers when you `#include` them. However, I can rename these remaining usages as well if you would like
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322313947
##########
libs/libc/stream/lib_bufferedoutstream.c:
##########
@@ -40,18 +40,18 @@
* Name: bufferedoutstream_flush
****************************************************************************/
-static int bufferedoutstream_flush(FAR struct lib_outstream_s *this)
+static int bufferedoutstream_flush(FAR struct lib_outstream_s *self)
{
- FAR struct lib_bufferedoutstream_s *rthis =
- (FAR struct lib_bufferedoutstream_s *)this;
+ FAR struct lib_bufferedoutstream_s *stream =
+ (FAR struct lib_bufferedoutstream_s *)self;
int ret = OK;
- ret = lib_stream_puts(rthis->backend, rthis->buffer,
- rthis->pending);
+ ret = lib_stream_puts(stream->backend, stream->buffer,
+ stream->pending);
if (ret >= 0)
{
- rthis->pending = 0;
+ stream->pending = 0;
Review Comment:
good catch, my editor added indents here for some reason
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713230274
> I've fixed a few missing updates and also created a PR in the nuttx-apps repo ([apache/nuttx-apps#2047](https://github.com/apache/nuttx-apps/pull/2047)) for the files there that need to be changed. I'm not sure what the workflow is like when changes need to be applied to both repos at the same time for either PR to pass
ci can't handle the dependence across multiple git yet, so the maintainer has to merge it after carefully reviewing.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #10565:
URL: https://github.com/apache/nuttx/pull/10565
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1327913425
##########
include/nuttx/streams.h:
##########
@@ -408,17 +402,17 @@ void lib_rawsostream(FAR struct lib_rawsostream_s *outstream, int fd);
* Wrap a raw output stream to a buffered output stream.
*
* Input Parameters:
- * outstream - User allocated, uninitialized instance of struct
- * lib_bufferedoutstream_s to be initialized.
- * backend - User allocated, initialized instance of struct
- * lib_outstream_s to be buffered.
+ * stream - User allocated, uninitialized instance of struct
+ * lib_bufferedoutstream_s to be initialized.
+ * backend - User allocated, initialized instance of struct
+* lib_outstream_s to be buffered.
Review Comment:
TODO: fix indent
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1713231761
@danielappiagyei-bc but the change from 'this' to 'self' lose?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] mu578 commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "mu578 (via GitHub)" <gi...@apache.org>.
mu578 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323347086
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
stream->handle
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1715807189
@danielappiagyei-bc please squash your change into one patch.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323311650
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
let's unify all ostream/istream/stdoutstream/stdistream to stream? @danielappiagyei-bc
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#issuecomment-1712975569
looks like I didn't properly rebase and squash into one commit. Will fix
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322310369
##########
libs/libc/stream/lib_bufferedoutstream.c:
##########
@@ -61,28 +61,28 @@ static int bufferedoutstream_flush(FAR struct lib_outstream_s *this)
* Name: bufferedoutstream_puts
****************************************************************************/
-static int bufferedoutstream_puts(FAR struct lib_outstream_s *this,
+static int bufferedoutstream_puts(FAR struct lib_outstream_s *self,
FAR const void *buf, int len)
{
- FAR struct lib_bufferedoutstream_s *rthis =
- (FAR struct lib_bufferedoutstream_s *)this;
+ FAR struct lib_bufferedoutstream_s *stream =
+ (FAR struct lib_bufferedoutstream_s *)self;
int ret = len;
- if (rthis->pending + len <= CONFIG_STREAM_OUT_BUFFER_SIZE)
+ if (stream->pending + len <= CONFIG_STREAM_OUT_BUFFER_SIZE)
{
/* If buffer is enough to save incoming data, cache it */
- memcpy(rthis->buffer + rthis->pending, buf, len);
- rthis->pending += len;
+ memcpy(stream->buffer + stream->pending, buf, len);
+ stream->pending += len;
Review Comment:
ditto
##########
libs/libc/stream/lib_bufferedoutstream.c:
##########
@@ -40,18 +40,18 @@
* Name: bufferedoutstream_flush
****************************************************************************/
-static int bufferedoutstream_flush(FAR struct lib_outstream_s *this)
+static int bufferedoutstream_flush(FAR struct lib_outstream_s *self)
{
- FAR struct lib_bufferedoutstream_s *rthis =
- (FAR struct lib_bufferedoutstream_s *)this;
+ FAR struct lib_bufferedoutstream_s *stream =
+ (FAR struct lib_bufferedoutstream_s *)self;
int ret = OK;
- ret = lib_stream_puts(rthis->backend, rthis->buffer,
- rthis->pending);
+ ret = lib_stream_puts(stream->backend, stream->buffer,
+ stream->pending);
if (ret >= 0)
{
- rthis->pending = 0;
+ stream->pending = 0;
Review Comment:
remove the extra indent
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -70,18 +71,19 @@ static void stdsostream_putc(FAR struct lib_sostream_s *self, int ch)
static int stdsostream_puts(FAR struct lib_sostream_s *self,
FAR const void *buffer, int len)
{
- FAR struct lib_stdsostream_s *stdsostream = (FAR struct lib_stdsostream_s *)self;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
why change
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -106,10 +108,11 @@ static int stdsostream_puts(FAR struct lib_sostream_s *self,
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *stdsostream = (FAR struct lib_stdsostream_s *)self;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
why change
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -120,10 +123,11 @@ static int stdsostream_flush(FAR struct lib_sostream_s *self)
static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *stdsostream = (FAR struct lib_stdsostream_s *)self;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
why change
##########
libs/libc/stream/lib_hexdumpstream.c:
##########
@@ -77,15 +77,15 @@ static size_t bin2hex(FAR const uint8_t *buf, size_t buflen,
* Name: hexdumpstream_flush
****************************************************************************/
-static int hexdumpstream_flush(FAR struct lib_outstream_s *this)
+static int hexdumpstream_flush(FAR struct lib_outstream_s *self)
{
- FAR struct lib_hexdumpstream_s *rthis = (FAR void *)this;
+ FAR struct lib_hexdumpstream_s *stream = (FAR void *)self;
- if (rthis->pending > 0)
+ if (stream->pending > 0)
{
- rthis->buffer[rthis->pending] = '\n';
- lib_stream_puts(rthis->backend, rthis->buffer, rthis->pending + 1);
- rthis->pending = 0;
+ stream->buffer[stream->pending] = '\n';
Review Comment:
ditto
##########
arch/arm64/src/common/arm64_initialize.c:
##########
@@ -145,7 +145,7 @@ static void up_color_intstack(void)
****************************************************************************/
#ifdef CONFIG_ARCH_FPU
-int arm64_panic_disable_fpu(struct notifier_block *this,
+int arm64_panic_disable_fpu(struct notifier_block *panicblock,
Review Comment:
change to block
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323320636
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
sure, consistency is good. I was worried `stream->stream` would look awkward but on second thought, it should be fine
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: streams.h: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1322325916
##########
libs/libc/stream/lib_bufferedoutstream.c:
##########
@@ -40,18 +40,18 @@
* Name: bufferedoutstream_flush
****************************************************************************/
-static int bufferedoutstream_flush(FAR struct lib_outstream_s *this)
+static int bufferedoutstream_flush(FAR struct lib_outstream_s *self)
{
- FAR struct lib_bufferedoutstream_s *rthis =
- (FAR struct lib_bufferedoutstream_s *)this;
+ FAR struct lib_bufferedoutstream_s *stream =
+ (FAR struct lib_bufferedoutstream_s *)self;
int ret = OK;
- ret = lib_stream_puts(rthis->backend, rthis->buffer,
- rthis->pending);
+ ret = lib_stream_puts(stream->backend, stream->buffer,
+ stream->pending);
if (ret >= 0)
{
- rthis->pending = 0;
+ stream->pending = 0;
Review Comment:
i've now reviewed my changes and fixed the extra indents in all files I modified, not just the ones you mentioned. good eye!
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] danielappiagyei-bc commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "danielappiagyei-bc (via GitHub)" <gi...@apache.org>.
danielappiagyei-bc commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323834354
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
I like that idea, I'll get started
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] mu578 commented on a diff in pull request #10565: c++ compatibility: rename reserved C++ identifiers 'public' and 'this'
Posted by "mu578 (via GitHub)" <gi...@apache.org>.
mu578 commented on code in PR #10565:
URL: https://github.com/apache/nuttx/pull/10565#discussion_r1323347086
##########
libs/libc/stream/lib_stdsostream.c:
##########
@@ -104,26 +106,28 @@ static int stdsostream_puts(FAR struct lib_sostream_s *this,
****************************************************************************/
#ifndef CONFIG_STDIO_DISABLE_BUFFERING
-static int stdsostream_flush(FAR struct lib_sostream_s *this)
+static int stdsostream_flush(FAR struct lib_sostream_s *self)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
+ (FAR struct lib_stdsostream_s *)self;
- DEBUGASSERT(sthis != NULL && sthis->stream != NULL);
- return lib_fflush(sthis->stream, true);
+ DEBUGASSERT(ostream != NULL && ostream->stream != NULL);
+ return lib_fflush(ostream->stream, true);
}
#endif
/****************************************************************************
* Name: stdsostream_seek
****************************************************************************/
-static off_t stdsostream_seek(FAR struct lib_sostream_s *this, off_t offset,
+static off_t stdsostream_seek(FAR struct lib_sostream_s *self, off_t offset,
int whence)
{
- FAR struct lib_stdsostream_s *sthis = (FAR struct lib_stdsostream_s *)this;
+ FAR struct lib_stdsostream_s *ostream =
Review Comment:
stream->handle, search and replace `->stream` `->handle` then rename struct field, maybe, also `.stream` `.handle` if any non-heap ref, it should be unlikely but you never know.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org