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/08/05 10:49:54 UTC

[GitHub] [incubator-nuttx-apps] masayuki2009 opened a new pull request, #1254: Modify telnetd

masayuki2009 opened a new pull request, #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254

   ## Summary
   
   - This PR contains the following commits
   - commit1: netutils: Make telnetd_daemon() in public
      - This commit makes telnetd_daemon() in public so that we
         can call it from applications.
     - Also, adds new configs to support posix_spawnp()
   - commit2: nshlib: Make nsh_telnetmain() in public
     - This commit makes nsh_telnetmain() in public so that we can
         call it from applications.
     - Also, CONFIG_NSH_DISABLE_TELNETSTART is introduced so that
         we can disable nsh_telnetstart()
   
   ## Impact
   
   - nsh with telnet daemon
   
   ## Testing
   
   - Tested with sabre-6quad:netnsh (will be updated later)
   


-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939440389


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

Review Comment:
   @xiaoxiang781216 
   So do you mean that `telnetd_start` should create 'Telnet daemon' with `posix_spawnp` as well?
   



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939307862


##########
include/netutils/telnetd.h:
##########
@@ -77,6 +91,23 @@ extern "C"
 #define EXTERN extern
 #endif
 
+/****************************************************************************
+ * Name: telnetd_daemon
+ *
+ * Description:
+ *   This function is the Telnet daemon.  It does not return (unless an
+ *   error occurs).
+ *
+ * Parameters:
+ *   Standard task start up arguments.
+ *
+ * Return:
+ *   Does not return unless an error occurs.
+ *
+ ****************************************************************************/
+
+int telnetd_daemon(int argc, FAR char *argv[]);

Review Comment:
   >who will do telnetd_daemon?
   
   I will create a new PR later where `examples/telnetd/telnetd.c` calls `telnetd_daemon`.
   



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940867307


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])

Review Comment:
   @xiaoxiang781216 
   >let's move telnetd example here instead since it contain more feature:
   
   Hmm, I don't think so, it just shows a dummy shell.
   Of course, we can modify it to spawn the standard shell.
   However, the code would have many `#ifdef`.
   That's why I implemented `system/telnetd`.



-- 
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-apps] pkarashchenko commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r938707914


##########
include/nshlib/nshlib.h:
##########
@@ -126,6 +126,27 @@ void nsh_initialize(void);
 
 int nsh_consolemain(int argc, FAR char *argv[]);
 
+/****************************************************************************
+ * Name: nsh_telnetmain
+ *
+ * Description:
+ *   This interface may be called or started with task_start to start a
+ *   single NSH instance that operates on stdin and stdout for telnet daemon.
+ *   This function does not return.
+ *
+ * Input Parameters:
+ *   Standard task start-up arguments.  These are not used.  argc may be
+ *   zero and argv may be NULL.
+ *
+ * Returned Values:
+ *   This function does not normally return.  exit() is usually called to
+ *   terminate the NSH session.  This function will return in the event of
+ *   an error.  In that case, a non-zero value is returned (EXIT_FAILURE=1).
+
+ ****************************************************************************/
+
+int nsh_telnetmain(int argc, char *argv[]);

Review Comment:
   ```suggestion
   int nsh_telnetmain(int argc, FAR char *argv[]);
   ```



##########
nshlib/nsh_telnetd.c:
##########
@@ -59,14 +59,14 @@ enum telnetd_state_e
 };
 
 /****************************************************************************
- * Private Functions
+ * Public Functions
  ****************************************************************************/
 
 /****************************************************************************
  * Name: nsh_telnetmain
  ****************************************************************************/
 
-static int nsh_telnetmain(int argc, char *argv[])
+int nsh_telnetmain(int argc, char *argv[])

Review Comment:
   ```suggestion
   int nsh_telnetmain(int argc, FAR char *argv[])
   ```



-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208892445

   > > @xiaoxiang781216 Could you review this PR again?
   > 
   > Let's summary my proposal:
   > 
   >     1. Move examples/telnetd to system/telnetd
   
   I will remove `examples/telnetd` later.
   
   > 
   >     2. Change telnetd_start to telnetd_run
   >        a. Remove task_create in telnetd_start
   >        b. Reuse the caller(telnetd) thread do the real work
   > 
   >     3. Replace telnetd_start in nsh to nsh_execute("telnetd"...)
   >
   > So the difference between flat and kernel mode is totally handled by nsh_execute.
   
   I'm a bit confusing.
   As you can see in https://github.com/apache/incubator-nuttx-apps/blob/e04333c986a47ec03ccb83e985f520fe30e3f1b6/nshlib/nsh_parse.c#L132 to call `nsh_execute`, we need to pass `FAR struct nsh_vtbl_s *vtbl` as well as `FAR const char *redirfile, int oflags)`. That's would be a big modification. 
   
   
   
   
   
   
   
   


-- 
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-apps] xiaoxiang781216 merged pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254


-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1206531563

   Hmm, I'm not sure why the following errors happen.
   
   ```
   ====================================================================================
   Configuration/Tool: sama5d4-ek/elf,CONFIG_ARMV7M_TOOLCHAIN_GNU_EABI
   ------------------------------------------------------------------------------------
     Cleaning...
     Configuring...
     Disabling CONFIG_ARMV7A_TOOLCHAIN_GNU_EABI
     Enabling CONFIG_ARMV7M_TOOLCHAIN_GNU_EABI
     Building NuttX...
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:55: errno] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: errno_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:55: hello] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: hello_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:54: struct] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: struct_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:55: signal] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: signal_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:99: hello++1] Error 1
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:103: hello++2] Error 1
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:110: hello++3] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: helloxx_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:55: mutex] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: mutex_install] Error 2
   arm-none-eabi-ld: cannot find -lproxies
   make[5]: *** [Makefile:55: pthread] Error 1
   make[5]: Target 'install' not remade because of errors.
   make[4]: *** [Makefile:71: pthread_install] Error 2
   make[4]: Target 'all' not remade because of errors.
   make[3]: *** [Makefile:59: build] Error 2
   make[3]: Target 'all' not remade because of errors.
   make[2]: *** [Makefile:42: /github/workspace/sources/apps/examples/elf_all] Error 2
   make[2]: Target '/github/workspace/sources/apps/libapps.a' not remade because of errors.
   make[1]: *** [Makefile:36: all] Error 2
   make: *** [tools/LibTargets.mk:210: /github/workspace/sources/apps/libapps.a] Error 2
   ```


-- 
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-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r938939669


##########
include/nshlib/nshlib.h:
##########
@@ -126,6 +126,27 @@ void nsh_initialize(void);
 
 int nsh_consolemain(int argc, FAR char *argv[]);
 
+/****************************************************************************
+ * Name: nsh_telnetmain
+ *
+ * Description:
+ *   This interface may be called or started with task_start to start a
+ *   single NSH instance that operates on stdin and stdout for telnet daemon.
+ *   This function does not return.
+ *
+ * Input Parameters:
+ *   Standard task start-up arguments.  These are not used.  argc may be
+ *   zero and argv may be NULL.
+ *
+ * Returned Values:
+ *   This function does not normally return.  exit() is usually called to
+ *   terminate the NSH session.  This function will return in the event of
+ *   an error.  In that case, a non-zero value is returned (EXIT_FAILURE=1).
+
+ ****************************************************************************/
+
+int nsh_telnetmain(int argc, FAR char *argv[]);

Review Comment:
   who will call nsh_telnetmain?



##########
include/netutils/telnetd.h:
##########
@@ -77,6 +91,23 @@ extern "C"
 #define EXTERN extern
 #endif
 
+/****************************************************************************
+ * Name: telnetd_daemon
+ *
+ * Description:
+ *   This function is the Telnet daemon.  It does not return (unless an
+ *   error occurs).
+ *
+ * Parameters:
+ *   Standard task start up arguments.
+ *
+ * Return:
+ *   Does not return unless an error occurs.
+ *
+ ****************************************************************************/
+
+int telnetd_daemon(int argc, FAR char *argv[]);

Review Comment:
   who will do telnetd_daemon?



##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

Review Comment:
   why not change telnetd to a normal application to handle both flat and kernel mode in the unified way?



-- 
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-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939496139


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

Review Comment:
   > @xiaoxiang781216 So do you mean that `telnetd_start` should create 'Telnet daemon' with `posix_spawnp` as well? 
   
   Yes.
   
   > However, `telnetd_start` is just a helper function and I think this API will not be used for CONFIG_BUILD_KERNEL=y because the daemon will be created on the prompt or in the script.
   
   We can change telnetd_start to telnetd_main and call task_create/posix_spawnp in both flat/kernel/protect build. It doesn't make sense to use the different method to launch telnetd.



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940888720


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])
+{
+  FAR char *argv1[3];
+  char arg0[sizeof("0x1234567812345678")];
+  FAR struct telnetd_s daemon;
+
+  /* Initialize the daemon structure */
+
+  daemon.port      = HTONS(23);
+  daemon.family    = AF_INET;
+  daemon.entry     = nsh_telnetmain;
+
+  /* NOTE: Settings for telnet session task */
+
+  daemon.priority  = CONFIG_SYSTEM_TELNETD_SESSION_PRIORITY;
+  daemon.stacksize = CONFIG_SYSTEM_TELNETD_SESSION_STACKSIZE;
+
+  snprintf(arg0, sizeof(arg0), "0x%" PRIxPTR, (uintptr_t)&daemon);

Review Comment:
   @pkarashchenko 
   The code was derived from https://github.com/apache/incubator-nuttx-apps/blob/e04333c986a47ec03ccb83e985f520fe30e3f1b6/netutils/telnetd/telnetd_daemon.c#L397
   



-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1209103082

   > Ok, let fix @pkarashchenko 's comment.
   
   @xiaoxiang781216 
   I've just fixed them.
   


-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1206961444

   Hmm, I need to fix the following error
   
   ```
   ====================================================================================
   Configuration/Tool: arty_a7/netnsh
   ------------------------------------------------------------------------------------
     Cleaning...
     Configuring...
     Building NuttX...
     % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                    Dload  Upload   Total   Spent    Left  Speed
   
     0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
     0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
   
     8  496k    8 44373    0     0   136k      0  0:00:03 --:--:--  0:00:03  136k
   100  496k  100  496k    0     0  1231k      0 --:--:-- --:--:-- --:--:-- 5266k
   riscv64-unknown-elf-ld: /github/workspace/sources/nuttx/staging/libapps.a(nsh_main.c.github.workspace.sources.apps.system.nsh.o): in function `nsh_main':
   /github/workspace/sources/apps/system/nsh/nsh_main.c:128: undefined reference to `nsh_telnetstart'
   ```


-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939307862


##########
include/netutils/telnetd.h:
##########
@@ -77,6 +91,23 @@ extern "C"
 #define EXTERN extern
 #endif
 
+/****************************************************************************
+ * Name: telnetd_daemon
+ *
+ * Description:
+ *   This function is the Telnet daemon.  It does not return (unless an
+ *   error occurs).
+ *
+ * Parameters:
+ *   Standard task start up arguments.
+ *
+ * Return:
+ *   Does not return unless an error occurs.
+ *
+ ****************************************************************************/
+
+int telnetd_daemon(int argc, FAR char *argv[]);

Review Comment:
   >who will do telnetd_daemon?
   
   @xiaoxiang781216 
   I will create a new PR later where `examples/telnetd/telnetd.c` calls `telnetd_daemon`.
   



-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1209017396

   >Yes, we just need launch telnetd from nsh like other normal program. 
   >To get vtbl, we may need move the launch to a late place.
   
   @xiaoxiang781216 
   I noticed that `nsh_system` could be used to execute `telnetd`. 
   However, it does not work correctly.
   
   >The key issue you encounter is that telnetd_start launch the background task by task_create, which doesn't work in kernel environment.
   
   Yes, and the current PR works for both FLAT and KERNEL types (PROTECTED type should work).
   So my proposal is to merge this PR first and apply your ideas later.
   
   What do you think?
   
   


-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208691964

   @xiaoxiang781216 
   Could you review this PR again?
   


-- 
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-apps] xiaoxiang781216 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208902556

   > I'm a bit confusing. As you can see in
   > 
   > https://github.com/apache/incubator-nuttx-apps/blob/e04333c986a47ec03ccb83e985f520fe30e3f1b6/nshlib/nsh_parse.c#L132
   > 
   > to call `nsh_execute`, we need to pass `FAR struct nsh_vtbl_s *vtbl`
   
   
   It's easy to get vtbl in libnsh
   
   > as well as `FAR const char *redirfile, int oflags)`. That's would be a big modification.
   
   both arguments can simply pass NULL/0 in this case.


-- 
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-apps] pkarashchenko commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r941606052


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])
+{
+  FAR char *argv1[3];
+  char arg0[sizeof("0x1234567812345678")];
+  FAR struct telnetd_s daemon;
+
+  /* Initialize the daemon structure */
+
+  daemon.port      = HTONS(23);
+  daemon.family    = AF_INET;
+  daemon.entry     = nsh_telnetmain;
+
+  /* NOTE: Settings for telnet session task */
+
+  daemon.priority  = CONFIG_SYSTEM_TELNETD_SESSION_PRIORITY;
+  daemon.stacksize = CONFIG_SYSTEM_TELNETD_SESSION_STACKSIZE;
+
+  snprintf(arg0, sizeof(arg0), "0x%" PRIxPTR, (uintptr_t)&daemon);

Review Comment:
   No problem, just curious;)



-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1206957311

   >Hmm, I'm not sure why the following errors happen.
   
   It seems that restarting the CI fixed the issue.
   So, the issue might be a parallel build issue.
   


-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r938727307


##########
include/nshlib/nshlib.h:
##########
@@ -126,6 +126,27 @@ void nsh_initialize(void);
 
 int nsh_consolemain(int argc, FAR char *argv[]);
 
+/****************************************************************************
+ * Name: nsh_telnetmain
+ *
+ * Description:
+ *   This interface may be called or started with task_start to start a
+ *   single NSH instance that operates on stdin and stdout for telnet daemon.
+ *   This function does not return.
+ *
+ * Input Parameters:
+ *   Standard task start-up arguments.  These are not used.  argc may be
+ *   zero and argv may be NULL.
+ *
+ * Returned Values:
+ *   This function does not normally return.  exit() is usually called to
+ *   terminate the NSH session.  This function will return in the event of
+ *   an error.  In that case, a non-zero value is returned (EXIT_FAILURE=1).
+
+ ****************************************************************************/
+
+int nsh_telnetmain(int argc, char *argv[]);

Review Comment:
   done



##########
nshlib/nsh_telnetd.c:
##########
@@ -59,14 +59,14 @@ enum telnetd_state_e
 };
 
 /****************************************************************************
- * Private Functions
+ * Public Functions
  ****************************************************************************/
 
 /****************************************************************************
  * Name: nsh_telnetmain
  ****************************************************************************/
 
-static int nsh_telnetmain(int argc, char *argv[])
+int nsh_telnetmain(int argc, char *argv[])

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] [incubator-nuttx-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939782536


##########
include/netutils/telnetd.h:
##########
@@ -77,6 +91,23 @@ extern "C"
 #define EXTERN extern
 #endif
 
+/****************************************************************************
+ * Name: telnetd_daemon
+ *
+ * Description:
+ *   This function is the Telnet daemon.  It does not return (unless an
+ *   error occurs).
+ *
+ * Parameters:
+ *   Standard task start up arguments.
+ *
+ * Return:
+ *   Does not return unless an error occurs.
+ *
+ ****************************************************************************/
+
+int telnetd_daemon(int argc, FAR char *argv[]);

Review Comment:
   I've added `system/telnetd` instead of modifying `examples/telnetd`.
   



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939312351


##########
include/nshlib/nshlib.h:
##########
@@ -126,6 +126,27 @@ void nsh_initialize(void);
 
 int nsh_consolemain(int argc, FAR char *argv[]);
 
+/****************************************************************************
+ * Name: nsh_telnetmain
+ *
+ * Description:
+ *   This interface may be called or started with task_start to start a
+ *   single NSH instance that operates on stdin and stdout for telnet daemon.
+ *   This function does not return.
+ *
+ * Input Parameters:
+ *   Standard task start-up arguments.  These are not used.  argc may be
+ *   zero and argv may be NULL.
+ *
+ * Returned Values:
+ *   This function does not normally return.  exit() is usually called to
+ *   terminate the NSH session.  This function will return in the event of
+ *   an error.  In that case, a non-zero value is returned (EXIT_FAILURE=1).
+
+ ****************************************************************************/
+
+int nsh_telnetmain(int argc, FAR char *argv[]);

Review Comment:
   >who will call nsh_telnetmain?
   
   @xiaoxiang781216 
   Same as above.



-- 
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-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940869322


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])

Review Comment:
   Ok, I think we simply drop example/telnetd, system/telnetd is enough to demo the usage.



-- 
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-apps] xiaoxiang781216 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208875452

   > @xiaoxiang781216 Could you review this PR again?
   
   Let's summary my proposal:
   
   1. Move examples/telnetd to system/telnetd
   2. Change telnetd_start to telnetd_run
       a. Remove task_create in telnetd_start
       b. Reuse the caller(telnetd) thread do the real work
   3. Replace telnetd_start in nsh to nsh_execute("telnetd"...)


-- 
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-apps] xiaoxiang781216 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208975732

   > > It's easy to get vtbl in libnsh.
   > 
   > @xiaoxiang781216 Hmm, I'm still confusing. Do you mean that we should use `nsh_newconsole()` `nsh_session()` `nsh_exit()` to execute the `telnetd`?
   
   Yes, we just need launch `telnetd` from nsh like other normal program.
   
   The key issue you encounter is that telnetd_start launch the background task by task_create, which doesn't work in kernel environment.
   
   The better fix is that we follow Unix server practice that:
   
   1. Server doesn't spawn/create task
   2. Let's shell or nsh spawn/create telnetd instead


-- 
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-apps] xiaoxiang781216 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1209022910

   Ok, let fix @pkarashchenko 's comment.


-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939783123


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

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] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940961938


##########
system/nsh/nsh_main.c:
##########
@@ -127,7 +127,8 @@ int main(int argc, FAR char *argv[])
 
   nsh_initialize();
 
-#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NETINIT_NETLOCAL)
+#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NSH_DISABLE_TELNETSTART) && \

Review Comment:
   Hmm, we need move nsh_telnetstart a little bit later to get vtbl context.



-- 
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-apps] pkarashchenko commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940852286


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -316,6 +307,33 @@ static int telnetd_daemon(int argc, FAR char *argv[])
           close(drvrfd);
         }
 
+#ifdef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
+      posix_spawn_file_actions_t file_actions;
+      posix_spawnattr_t attr;
+      FAR char *argv1[2];

Review Comment:
   Local variables should be defined at the beginning of the scope (C89 compatibility).



##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])
+{
+  FAR char *argv1[3];
+  char arg0[sizeof("0x1234567812345678")];
+  FAR struct telnetd_s daemon;

Review Comment:
   ```suggestion
     struct telnetd_s daemon;
   ```
   



##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])
+{
+  FAR char *argv1[3];
+  char arg0[sizeof("0x1234567812345678")];
+  FAR struct telnetd_s daemon;
+
+  /* Initialize the daemon structure */
+
+  daemon.port      = HTONS(23);
+  daemon.family    = AF_INET;
+  daemon.entry     = nsh_telnetmain;
+
+  /* NOTE: Settings for telnet session task */
+
+  daemon.priority  = CONFIG_SYSTEM_TELNETD_SESSION_PRIORITY;
+  daemon.stacksize = CONFIG_SYSTEM_TELNETD_SESSION_STACKSIZE;
+
+  snprintf(arg0, sizeof(arg0), "0x%" PRIxPTR, (uintptr_t)&daemon);

Review Comment:
   Why not just `%p`?



-- 
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-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940855300


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])

Review Comment:
   let's move telnetd example here instead since it contain more feature:
   https://github.com/apache/incubator-nuttx-apps/tree/master/examples/telnetd



##########
system/nsh/nsh_main.c:
##########
@@ -127,7 +127,8 @@ int main(int argc, FAR char *argv[])
 
   nsh_initialize();
 
-#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NETINIT_NETLOCAL)
+#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NSH_DISABLE_TELNETSTART) && \

Review Comment:
   let's  call nsh_execute to launch system/telnetd in nsh_telnetstart



##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -316,6 +307,33 @@ static int telnetd_daemon(int argc, FAR char *argv[])
           close(drvrfd);
         }
 
+#ifdef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP

Review Comment:
   don't need spawn, we can reuse the caller thread(telnetd main thread) as the context.



-- 
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-apps] xiaoxiang781216 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940869322


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])

Review Comment:
   Ok, I think we simply drop example/telnetd.



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r940884532


##########
system/nsh/nsh_main.c:
##########
@@ -127,7 +127,8 @@ int main(int argc, FAR char *argv[])
 
   nsh_initialize();
 
-#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NETINIT_NETLOCAL)
+#if defined(CONFIG_NSH_TELNET) && !defined(CONFIG_NSH_DISABLE_TELNETSTART) && \

Review Comment:
   @xiaoxiang781216 
   In my understanding, we can not call `nsh_execute` here.
   



-- 
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-apps] masayuki2009 commented on pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#issuecomment-1208913336

   >It's easy to get vtbl in libnsh.
   
   @xiaoxiang781216 
   Hmm, I'm still confusing. Do you mean that we should use `nsh_newconsole()` `nsh_session()` `nsh_exit()` to execute the `telnetd`?
   
   


-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r941079832


##########
system/telnetd/telnetd.c:
##########
@@ -0,0 +1,61 @@
+/****************************************************************************
+ * apps/system/telnetd/telnetd.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "netutils/telnetd.h"
+#include "netutils/netlib.h"
+#include "nshlib/nshlib.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char *argv[])
+{
+  FAR char *argv1[3];
+  char arg0[sizeof("0x1234567812345678")];
+  FAR struct telnetd_s daemon;

Review Comment:
   done.
   



##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -316,6 +307,33 @@ static int telnetd_daemon(int argc, FAR char *argv[])
           close(drvrfd);
         }
 
+#ifdef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
+      posix_spawn_file_actions_t file_actions;
+      posix_spawnattr_t attr;
+      FAR char *argv1[2];

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] [incubator-nuttx-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939521889


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

Review Comment:
   OK. I will try to modify `telnetd_start` later.
   
   



-- 
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-apps] masayuki2009 commented on a diff in pull request #1254: Modify telnetd

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1254:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1254#discussion_r939440389


##########
netutils/telnetd/telnetd_daemon.c:
##########
@@ -369,6 +389,7 @@ static int telnetd_daemon(int argc, FAR char *argv[])
  *
  ****************************************************************************/
 
+#ifndef CONFIG_NETUTILS_TELNETD_USE_POSIX_SPAWNP
 int telnetd_start(FAR struct telnetd_config_s *config)

Review Comment:
   @xiaoxiang781216 
   So do you mean that `telnetd_start` should create 'Telnet daemon' with `posix_spawnp` as well?
   However, `telnetd_start` is just a helper function and I think this API will not be used for CONFIG_BUILD_KERNEL=y because the daemon will be created on the prompt or in the script.
   



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