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/09/17 04:06:29 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

xiaoxiang781216 opened a new pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566


   ## Summary
   since some drivers(e.g. semihosting) have more fast implementation.
   
   ## Impact
   More fast speed in some special case(e.g. semihosting)
   
   ## 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] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923851046


   > The commit [ca62341](https://github.com/apache/incubator-nuttx/commit/ca62341c931a2ee2536aea9d8f598f4e9f11f833) is incorrect and should be reverted. Nothing guarantees that the buffer is NUL-terminated. Nothing guarantees there won't be multiple NUL-terminated C-strings in the buffer. That is the reason for the length parameter in the syslog API. With this change, I started getting garbage at beginning of syslog lines:
   > 
   
   @juniskane sorry for inconvenience. Do you think the change in #4580 is good or add len parameter to up_puts?


-- 
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] gustavonihei commented on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923858972


   According to the [syslog documentation](https://pubs.opengroup.org/onlinepubs/007904975/functions/syslog.html):
   
   > The message body is generated from the message and following arguments in the same manner as if these were arguments to printf()
   
   and according to the [printf` documentation](https://en.cppreference.com/w/c/io/fprintf):
   
   > format - pointer to a null-terminated multibyte string specifying how to interpret the data
   
   So, if my analysis is correct, the `syslog` API assumes that the provided string buffer is always NUL-terminated.
   
   


-- 
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] juniskane commented on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923830708


   The commit ca62341c931a2ee2 is incorrect and should be reverted. Nothing guarantees that the buffer is NUL-terminated. Nothing guarantees there won't be multiple NUL-terminated C-strings in the buffer. That is the reason for the length parameter in the syslog API. With this change, I started getting garbage at beginning of syslog lines:
   
   ```
   �7r�z+�.Xٰ������R��2�L;%��H��S�lَ������(B2w's<�/�����[1632217938.702673] wirepas_stack_start: Wait for WP FW to start...
   TTPing after MQTTPublish (AWS GG workaround).
   �7r�z+�.Xٰ������R��2�L;%��H��S�lَ������(B2w's<�/�����[1632217938.762673] cycle: Received MQTT Ping response!
   ��[��[1632217938.762673] handle_event: SEND_DONE [RUNNING]
   �D�U���Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217938.762673] handle_event: network connected : 200
   ING]
   �D�U���Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217938.762673] handle_event: PING [RUNNING]
   �D�U���Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217938.982673] Platform_LOG: [msap] I: Status is 0x01
   �D�U���Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217939.732673] Platform_LOG: [msap] I: Stop request result = 0x00
   K�D�U���Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217940.242673] Platform_LOG: [SLIP] W: Timeout to receive frame (size=0)
   ��Q�F*�`��0�NQ�4�3�v�vg�֞j[1632217940.242673] Platform_LOG: [wpc_int] E: Didn't receive answer to the request 0x0c
   �0�NQ�4�3�v�vg�֞j[1632217940.282673] Platform_LOG: [msap] I: Status is 0x01
   ```
   


-- 
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] gustavonihei edited a comment on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923865313


   > According to the [syslog documentation](https://pubs.opengroup.org/onlinepubs/007904975/functions/syslog.html):
   > 
   > > The message body is generated from the message and following arguments in the same manner as if these were arguments to printf()
   > 
   > and according to the [printf documentation](https://en.cppreference.com/w/c/io/fprintf):
   > 
   > > format - pointer to a null-terminated multibyte string specifying how to interpret the data
   > 
   > So, if my analysis is correct, the `syslog` API assumes that the provided string buffer is always NUL-terminated.
   
   I just noticed the https://github.com/apache/incubator-nuttx/pull/4580 PR from @xiaoxiang781216.
   My analysis might be correct, but that is not where the issue lies then.
   Indeed, if the intermediate buffer is not null-terminated, garbage will be printed.


-- 
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] gustavonihei edited a comment on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
gustavonihei edited a comment on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923858972


   According to the [syslog documentation](https://pubs.opengroup.org/onlinepubs/007904975/functions/syslog.html):
   
   > The message body is generated from the message and following arguments in the same manner as if these were arguments to printf()
   
   and according to the [printf documentation](https://en.cppreference.com/w/c/io/fprintf):
   
   > format - pointer to a null-terminated multibyte string specifying how to interpret the data
   
   So, if my analysis is correct, the `syslog` API assumes that the provided string buffer is always NUL-terminated.
   
   


-- 
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 #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923851046


   > The commit [ca62341](https://github.com/apache/incubator-nuttx/commit/ca62341c931a2ee2536aea9d8f598f4e9f11f833) is incorrect and should be reverted. Nothing guarantees that the buffer is NUL-terminated. Nothing guarantees there won't be multiple NUL-terminated C-strings in the buffer. That is the reason for the length parameter in the syslog API. With this change, I started getting garbage at beginning of syslog lines:
   > 
   
   Sorry for inconvenience. Do you think the change in #4580 is good or add len parameter to up_puts?


-- 
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] juniskane commented on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-924621898


   I tested PR #4580 and it fixes this issue for me. My defconfig has CONFIG_SYSLOG_BUFFER, so root cause was within this syslog internal buffering, not related to public syslog() API.


-- 
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] gustavonihei merged pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566


   


-- 
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] gustavonihei commented on pull request #4566: drivers/syslog: Call up_puts in syslog_default_write instad up_putc

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #4566:
URL: https://github.com/apache/incubator-nuttx/pull/4566#issuecomment-923865313


   > According to the [syslog documentation](https://pubs.opengroup.org/onlinepubs/007904975/functions/syslog.html):
   > 
   > > The message body is generated from the message and following arguments in the same manner as if these were arguments to printf()
   > 
   > and according to the [printf documentation](https://en.cppreference.com/w/c/io/fprintf):
   > 
   > > format - pointer to a null-terminated multibyte string specifying how to interpret the data
   > 
   > So, if my analysis is correct, the `syslog` API assumes that the provided string buffer is always NUL-terminated.
   
   I just noticed the https://github.com/apache/incubator-nuttx/pull/4580 PR from @xiaoxiang781216.
   My analysis might be correct, but that is not where the issue lies then.
   


-- 
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