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/28 06:56:32 UTC
[GitHub] [nuttx] XinStellaris opened a new pull request, #7715: libc/stream:add putc interface for mtd/blk stream.
XinStellaris opened a new pull request, #7715:
URL: https://github.com/apache/nuttx/pull/7715
Signed-off-by: 田昕 <ti...@xiaomi.com>
## Summary
add putc interface for mtd/blk stream
## Impact
Should be none.
## Testing
ESP32C3 tested.
--
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 #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033660318
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -217,6 +227,7 @@ int lib_blkoutstream_open(FAR struct lib_blkoutstream_s *stream,
}
stream->inode = inode;
+ stream->public.put = blkoutstream_putc;
Review Comment:
It's little bit inconsistent in the original code base:
1. The user see the function as lib_stream_put or lib_outstream_s::put
2. The implementation normally name xxx_putc as statical function
--
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 #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033660318
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -217,6 +227,7 @@ int lib_blkoutstream_open(FAR struct lib_blkoutstream_s *stream,
}
stream->inode = inode;
+ stream->public.put = blkoutstream_putc;
Review Comment:
It's little bit inconsistent in the original code base:
1. The user see the function as lib_stream_put or lib_outstream_s::put
2. The implementation normally name xxx_putc as statical function
Do you think we should make the user and implementor consistent with each other?
--
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 #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033660318
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -217,6 +227,7 @@ int lib_blkoutstream_open(FAR struct lib_blkoutstream_s *stream,
}
stream->inode = inode;
+ stream->public.put = blkoutstream_putc;
Review Comment:
It's little bit inconsistent:
1. The user see the function as lib_stream_put or lib_outstream_s::put
2. The implementation normally name xxx_putc as statical function
--
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] pkarashchenko commented on a diff in pull request #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033191940
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -127,6 +127,16 @@ static int blkoutstream_puts(FAR struct lib_outstream_s *this,
return len;
}
+/****************************************************************************
+ * Name: blkoutstream_putc
+ ****************************************************************************/
+
+static void blkoutstream_putc(FAR struct lib_outstream_s *this, int ch)
Review Comment:
Why not
```suggestion
static void blkoutstream_put(FAR struct lib_outstream_s *this, int ch)
```
?
--
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] XinStellaris commented on a diff in pull request #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033193598
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -127,6 +127,16 @@ static int blkoutstream_puts(FAR struct lib_outstream_s *this,
return len;
}
+/****************************************************************************
+ * Name: blkoutstream_putc
+ ****************************************************************************/
+
+static void blkoutstream_putc(FAR struct lib_outstream_s *this, int ch)
Review Comment:
I named them putc to be consistent.
--
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] XinStellaris commented on a diff in pull request #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033193380
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -127,6 +127,16 @@ static int blkoutstream_puts(FAR struct lib_outstream_s *this,
return len;
}
+/****************************************************************************
+ * Name: blkoutstream_putc
+ ****************************************************************************/
+
+static void blkoutstream_putc(FAR struct lib_outstream_s *this, int ch)
Review Comment:
Because other C code used the name of putc, such as memoutstream_putc, lowoutstream_putc
--
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 #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7715:
URL: https://github.com/apache/nuttx/pull/7715
--
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] pkarashchenko commented on a diff in pull request #7715: libc/stream:add putc interface for mtd/blk stream.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7715:
URL: https://github.com/apache/nuttx/pull/7715#discussion_r1033260901
##########
libs/libc/stream/lib_blkoutstream.c:
##########
@@ -217,6 +227,7 @@ int lib_blkoutstream_open(FAR struct lib_blkoutstream_s *stream,
}
stream->inode = inode;
+ stream->public.put = blkoutstream_putc;
Review Comment:
@xiaoxiang781216 maybe stream method also should be `putc`? What do you think? I've seen that recently you done some clean-up and changes from `putc` to `put`, but do not know the reasons why we stay with `put` now
--
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