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 2020/02/26 14:00:04 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
xiaoxiang781216 opened a new pull request #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382
it is impossible that syslog device get initialized before
since board_app_initialize call syslog_file_channel only once
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
Change-Id: Ia72c931f5e19f771e1864f63ede8565624c8c6b4
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591822072
I see no reason to restrict this to kernel only and that limitation is not in the API documentation. The ability to change syslog stream dynamically is *extremely useful* for testing. I can envision board level logic doing that after some BOARDIOC so it is not relevant if this function is called directly from app code or not. I currently use the "side effect" of (syslog internal) variables retaining their value in flat builds over multiple program invocations:
> const char *syslog_file;
> /* ... this is in app main(), ... get syslog_file from argv... */
>
> if (syslog_file)
> {
> #ifdef CONFIG_SYSLOG_FILE
> /* Convenience names. */
> if (strcmp(syslog_file, "usb") == 0)
> syslog_file = "/dev/ttyACM0";
> else if (strcmp(syslog_file, "uart") == 0)
> syslog_file = "/dev/ttyS3";
> if (syslog_file_channel(syslog_file) < 0)
> fprintf(stderr, "Cannot set syslog to %s\n", syslog_file);
> #else
> fprintf(stderr, "Cannot set syslog to %s\n", syslog_file);
> #endif
> exit(0);
> }
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591484547
Should I wait for Juha's comments to merge? I cannot select him as a reviewer.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591440035
Also fix a build erorr noted here:
https://github.com/apache/incubator-nuttx/pull/366
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591984773
> I have applications that call syslog_file_channel() based on run-time command-line arguments,
You know that that is a violation of the POSIX OS interface. We cannot be responsible for people's use of internal, OS standard OS functions. Don't count on that kind of portability in the future; we won't support 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591872715
@juniskane syslog_dev_uninitialize shouldn't return error in the 2nd patch if there isn't the initialization yet, please try the 3rd patch again, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane edited a comment on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane edited a comment on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591822072
I see no reason to restrict this to kernel only and that limitation is not in the API documentation. The ability to change syslog stream dynamically is *extremely useful* for testing. I can envision board level logic doing that after some BOARDIOC so it is not relevant if this function is called directly from app code or not. I currently use the "side effect" of (syslog internal) variables retaining their value in flat builds over multiple program invocations:
```
const char *syslog_file;
/* ... this is in app main(), ... get syslog_file from argv... */
if (syslog_file)
{
#ifdef CONFIG_SYSLOG_FILE
/* Convenience names. */
if (strcmp(syslog_file, "usb") == 0)
syslog_file = "/dev/ttyACM0";
else if (strcmp(syslog_file, "uart") == 0)
syslog_file = "/dev/ttyS3";
if (syslog_file_channel(syslog_file) < 0)
fprintf(stderr, "Cannot set syslog to %s\n", syslog_file);
#else
fprintf(stderr, "Cannot set syslog to %s\n", syslog_file);
#endif
exit(0);
}
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591441140
@juniskane Please try this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591516186
Sure, let's wait hist feedback until tomorrow.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591952795
@patacongo this patch is ready to merege 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591810891
> This is completely unacceptable. I have applications that call syslog_file_channel() based on run-time command-line arguments, to change the syslog destination on the fly. That is a useful functionality and there is no reason to limit this API to one-time initialization only. The patch causing build failure should be reverted instead i.e. do not make that one symbol static.
This API is for kernel use only, all mainline caller is from board_app_iniitialize, but you have a program call syslog_file_channel from userspace, do you have case launch this program multiple times?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591924495
Does not even compile, or am I missing an interim patch?
```
syslog/syslog_device.c: In function 'syslog_dev_outputready':
syslog/syslog_device.c:268:15: error: void value not ignored as it ought to be
ret = syslog_dev_initialize(g_syslog_dev.sl_devpath,
^
syslog/syslog_device.c: At top level:
syslog/syslog_device.c:316:5: error: conflicting types for 'syslog_dev_initialize'
int syslog_dev_initialize(FAR const char *devpath, int oflags, int mode)
^~~~~~~~~~~~~~~~~~~~~
In file included from syslog/syslog_device.c:59:0:
syslog/syslog.h:99:6: note: previous declaration of 'syslog_dev_initialize' was here
void syslog_dev_initialize(FAR const char *devpath, int oflags, int mode);
^~~~~~~~~~~~~~~~~~~~~
syslog/syslog_device.c:416:6: error: conflicting types for 'syslog_dev_uninitialize'
void syslog_dev_uninitialize(void)
^~~~~~~~~~~~~~~~~~~~~~~
In file included from syslog/syslog_device.c:59:0:
syslog/syslog.h:122:5: note: previous declaration of 'syslog_dev_uninitialize' was here
int syslog_dev_uninitialize(void);
^~~~~~~~~~~~~~~~~~~~~~~
Makefile:126: recipe for target 'syslog_device.o' failed
make[1]: *** [syslog_device.o] Error 1
make[1]: Leaving directory '/home/jni/u/yard/workarea/nuttx/drivers'
tools/LibTargets.mk:104: recipe for target 'drivers/libdrivers.a' failed
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591841994
This works otherwise, but not in the case syslog_file_channel() fails ever, even once. Cannot recover back to working device without reboot:
```
nsh> app -l /dev/nonexistent
Cannot set syslog to /dev/nonexistent
nsh> app -l usb
Cannot set syslog to /dev/ttyACM0
nsh> app -l uart
Cannot set syslog to /dev/ttyS3
nsh>
```
So it is still a regression of old, working functionality.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591931624
> Does not even compile, or am I missing an interim patch?
>
> ```
> syslog/syslog_device.c: In function 'syslog_dev_outputready':
> syslog/syslog_device.c:268:15: error: void value not ignored as it ought to be
> ret = syslog_dev_initialize(g_syslog_dev.sl_devpath,
> ^
> syslog/syslog_device.c: At top level:
> syslog/syslog_device.c:316:5: error: conflicting types for 'syslog_dev_initialize'
> int syslog_dev_initialize(FAR const char *devpath, int oflags, int mode)
> ^~~~~~~~~~~~~~~~~~~~~
> In file included from syslog/syslog_device.c:59:0:
> syslog/syslog.h:99:6: note: previous declaration of 'syslog_dev_initialize' was here
> void syslog_dev_initialize(FAR const char *devpath, int oflags, int mode);
> ^~~~~~~~~~~~~~~~~~~~~
> syslog/syslog_device.c:416:6: error: conflicting types for 'syslog_dev_uninitialize'
> void syslog_dev_uninitialize(void)
> ^~~~~~~~~~~~~~~~~~~~~~~
> In file included from syslog/syslog_device.c:59:0:
> syslog/syslog.h:122:5: note: previous declaration of 'syslog_dev_uninitialize' was here
> int syslog_dev_uninitialize(void);
> ^~~~~~~~~~~~~~~~~~~~~~~
> Makefile:126: recipe for target 'syslog_device.o' failed
> make[1]: *** [syslog_device.o] Error 1
> make[1]: Leaving directory '/home/jni/u/yard/workarea/nuttx/drivers'
> tools/LibTargets.mk:104: recipe for target 'drivers/libdrivers.a' failed
> ```
Sorry, I change the wrong function prototype, how about the new patch? It is hard to test my patch local since there isn't official config enabled this option.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591947244
Ok, now it seems be working as before.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #382:
syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591832703
@juniskane Ok, the new fix should address your concern, please take a look.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo merged pull request #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] juniskane commented on issue #382: syslog/file:
Remove syslog_dev_uninitialize in syslog_file_channel
Posted by GitBox <gi...@apache.org>.
juniskane commented on issue #382: syslog/file: Remove syslog_dev_uninitialize in syslog_file_channel
URL: https://github.com/apache/incubator-nuttx/pull/382#issuecomment-591804411
This is completely unacceptable. I have applications that call syslog_file_channel() based on run-time command-line arguments, to change the syslog destination on the fly. That is a useful functionality and there is no reason to limit this API to one-time initialization only. The patch causing build failure should be reverted instead i.e. do not make that one symbol static.
----------------------------------------------------------------
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
With regards,
Apache Git Services