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 2022/11/24 16:34:51 UTC
[GitHub] [incubator-nuttx] Gary-Hobson opened a new pull request, #7688: include: unified stream read and write interface
Gary-Hobson opened a new pull request, #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688
## Summary
Provide simple macros for unified stream read and write interfaces
## Impact
## Testing
--
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 #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7688:
URL: https://github.com/apache/nuttx/pull/7688
--
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] [incubator-nuttx] Gary-Hobson commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
Gary-Hobson commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031721935
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->puts \
+ (( FAR struct lib_outstream_s *)stream, buf, len)
+#define stream_putc(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->putc \
+ (( FAR struct lib_outstream_s *)stream, ch)
+#define stream_getc(stream) \
Review Comment:
What are the functions of nput and nget?
In all the code, I only see the value of modifying it, but no one has ever used 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Gary-Hobson commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
Gary-Hobson commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031745787
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
#define lib_stream_puts(stream, buf, len) \
_**(FAR struct lib_outstream_s *)**_(stream)->puts((FAR struct lib_outstream_s *)(stream), buf, len)
I have a little doubt about this code, which variable does it decorate (FAR struct lib_outstream_s *)
Does this lead to different results if the precedence of "->" and "()" is not the same in C and C++, as deduced by operator precedence.
这段代码我有一点疑问, (FAR struct lib_outstream_s *) 这个类型转换修饰是那一个变量呢？
如果按照运算符优先级进行推导，在 C 和 C++ 中 “->” 和 “()” 的优先级并不相同，这是否会导致不同的结果。
https://en.cppreference.com/w/c/language/cast
https://en.cppreference.com/w/cpp/language/operator_precedence
https://en.cppreference.com/w/c/language/operator_precedence
--
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] [incubator-nuttx] xiaoxiang781216 commented on pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#issuecomment-1327170323
please rebase to the last master
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031754583
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
The keypoint is you need add () around stream argument. The additional () is fine if there is doubt.
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031709402
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->puts \
+ (( FAR struct lib_outstream_s *)stream, buf, len)
+#define stream_putc(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->putc \
+ (( FAR struct lib_outstream_s *)stream, ch)
+#define stream_getc(stream) \
+ (( FAR struct lib_instream_s *)stream)->get \
+ (( FAR struct lib_instream_s *)stream)
Review Comment:
let's create a new patch to change all direct access to the new macro
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
change to:
```
#define lib_stream_puts(stream, buf, len) \
(FAR struct lib_outstream_s *)(stream)->puts( \
(FAR struct lib_outstream_s *)(stream), buf, len)
```
apply the similar change to the follow macro
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->puts \
+ (( FAR struct lib_outstream_s *)stream, buf, len)
+#define stream_putc(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->putc \
+ (( FAR struct lib_outstream_s *)stream, ch)
+#define stream_getc(stream) \
Review Comment:
what other wrapper e.g. seek nput, nget
--
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] [incubator-nuttx] Gary-Hobson commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
Gary-Hobson commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031753302
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
I have a little doubt about this code, (FAR struct lib_outstream_s *) Which variable is this type conversion decoration?
According to the deduction of operator precedence, "->" has a higher priority than "()", and it should be modified puts
But the type of puts is not the same as (FAR struct lib_outstream_s *), is this a problem
这段代码我有一点疑问, (FAR struct lib_outstream_s *) 这个类型转换修复是那一个变量呢
按照运算符优先级推导， “->” 的优先级高于 “()” ，它应该是修饰 puts
但是 puts 类型和(FAR struct lib_outstream_s *)并不相同，这是否存在问题
https://en.cppreference.com/w/c/language/cast
https://en.cppreference.com/w/c/language/operator_precedence
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031727829
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->puts \
+ (( FAR struct lib_outstream_s *)stream, buf, len)
+#define stream_putc(stream, buf, len) \
+ (( FAR struct lib_outstream_s *)stream)->putc \
+ (( FAR struct lib_outstream_s *)stream, ch)
+#define stream_getc(stream) \
Review Comment:
> What are the functions of nput and nget?
>
> In all the code, I only see the value of modifying it, but no one has ever used it
it can be used in the future. Please add gets macro: https://github.com/apache/incubator-nuttx/pull/7681
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031754583
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
The keypoint is you need add () around stream argument
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1032131324
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,25 @@
* Pre-processor Definitions
****************************************************************************/
+#define lib_stream_puts(stream, buf, len) \
+ ((FAR struct lib_outstream_s *)(stream))->puts( \
+ (FAR struct lib_outstream_s *)(stream), buf, len)
+#define lib_stream_putc(stream, buf, len) \
Review Comment:
```suggestion
#define lib_stream_putc(stream, 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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031726416
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
The change still wrong, please read my comment carefully.
--
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] [incubator-nuttx] Gary-Hobson commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
Gary-Hobson commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1031753302
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,16 @@
* Pre-processor Definitions
****************************************************************************/
+#define stream_write(stream, buf, len) \
Review Comment:
#define lib_stream_puts(stream, buf, len) \
**(FAR struct lib_outstream_s *)**(stream)->puts((FAR struct lib_outstream_s *)(stream), buf, len)
I have a little doubt about this code, (FAR struct lib_outstream_s *) Which variable is this type conversion decoration?
According to the deduction of operator precedence, "->" has a higher priority than "()", and it should be modified puts
But the type of puts is not the same as (FAR struct lib_outstream_s *), is this a problem
这段代码我有一点疑问, (FAR struct lib_outstream_s *) 这个类型转换修复是那一个变量呢
按照运算符优先级推导， “->” 的优先级高于 “()” ，它应该是修饰 puts
但是 puts 类型和(FAR struct lib_outstream_s *)并不相同，这是否存在问题
https://en.cppreference.com/w/c/language/cast
https://en.cppreference.com/w/c/language/operator_precedence
--
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] [incubator-nuttx] Gary-Hobson commented on a diff in pull request #7688: include: unified stream read and write interface
Posted by GitBox <gi...@apache.org>.
Gary-Hobson commented on code in PR #7688:
URL: https://github.com/apache/incubator-nuttx/pull/7688#discussion_r1032178943
##########
include/nuttx/streams.h:
##########
@@ -40,6 +40,25 @@
* Pre-processor Definitions
****************************************************************************/
+#define lib_stream_puts(stream, buf, len) \
+ ((FAR struct lib_outstream_s *)(stream))->puts( \
+ (FAR struct lib_outstream_s *)(stream), buf, len)
+#define lib_stream_putc(stream, buf, len) \
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