xiaoxiang781216 commented on code in PR #3090:
URL: https://github.com/apache/nuttx-apps/pull/3090#discussion_r2134952912


##########
system/syslogd/syslogd_main.c:
##########
@@ -0,0 +1,291 @@
+/****************************************************************************
+ * apps/system/syslogd/syslogd_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for asyslogditional 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 <nuttx/config.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <getopt.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <syslog.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Program name */
+
+#define g_syslogd CONFIG_SYSTEM_SYSLOGD_PROGNAME
+
+/* syslogd program version */
+
+#define SYSLOGD_VERSION "0.0.0"
+
+/* Minimum buffer size check */
+
+#if CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE < 480
+#error "SYSTEM_SYSLOGD_ENTRYSIZE must be more than 480 to satisfy RFC 5424"
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+bool g_debugmode = false;
+bool g_background = true;
+char g_buffer[CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE];

Review Comment:
   why not use local buffer



##########
system/syslogd/syslogd_main.c:
##########
@@ -0,0 +1,291 @@
+/****************************************************************************
+ * apps/system/syslogd/syslogd_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for asyslogditional 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 <nuttx/config.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <getopt.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <syslog.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Program name */
+
+#define g_syslogd CONFIG_SYSTEM_SYSLOGD_PROGNAME
+
+/* syslogd program version */
+
+#define SYSLOGD_VERSION "0.0.0"
+
+/* Minimum buffer size check */
+
+#if CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE < 480
+#error "SYSTEM_SYSLOGD_ENTRYSIZE must be more than 480 to satisfy RFC 5424"
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+bool g_debugmode = false;
+bool g_background = true;
+char g_buffer[CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: print_usage
+ ****************************************************************************/
+
+static void print_usage(void)
+{
+  fprintf(stderr, "usage:\n");
+  fprintf(stderr, "  %s [-vdn]\n", g_syslogd);
+}
+
+/****************************************************************************
+ * Name: read_log
+ *
+ * Description:
+ *   Read a single log (end indicated by a newline character) into `buf`.
+ ****************************************************************************/
+
+static ssize_t read_log(int fd, char *buf, size_t n)
+{
+  ssize_t count = 0;
+  ssize_t bread;
+
+  while (count < n)
+    {
+      bread = read(fd, buf, 1);

Review Comment:
   the performance is very poor if we read one byte one time



##########
system/syslogd/CMakeLists.txt:
##########
@@ -0,0 +1,36 @@
+# 
##############################################################################
+# apps/system/dd/CMakeLists.txt

Review Comment:
   dd->syslogd



##########
system/syslogd/syslogd_main.c:
##########
@@ -0,0 +1,291 @@
+/****************************************************************************
+ * apps/system/syslogd/syslogd_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for asyslogditional 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 <nuttx/config.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <getopt.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <syslog.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Program name */
+
+#define g_syslogd CONFIG_SYSTEM_SYSLOGD_PROGNAME
+
+/* syslogd program version */
+
+#define SYSLOGD_VERSION "0.0.0"
+
+/* Minimum buffer size check */
+
+#if CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE < 480
+#error "SYSTEM_SYSLOGD_ENTRYSIZE must be more than 480 to satisfy RFC 5424"
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+bool g_debugmode = false;
+bool g_background = true;
+char g_buffer[CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: print_usage
+ ****************************************************************************/
+
+static void print_usage(void)
+{
+  fprintf(stderr, "usage:\n");
+  fprintf(stderr, "  %s [-vdn]\n", g_syslogd);
+}
+
+/****************************************************************************
+ * Name: read_log
+ *
+ * Description:
+ *   Read a single log (end indicated by a newline character) into `buf`.
+ ****************************************************************************/
+
+static ssize_t read_log(int fd, char *buf, size_t n)
+{
+  ssize_t count = 0;
+  ssize_t bread;
+
+  while (count < n)
+    {
+      bread = read(fd, buf, 1);
+
+      /* Error occurred */
+
+      if (bread < 0)
+        {
+          return bread;
+        }
+
+      /* End of stream reached */
+
+      if (bread == 0)
+        {
+          break;
+        }
+
+      /* Newline character reached */
+
+      if (*buf == '\n')
+        {
+          /* Don't increment count so user 'skips' the '\n' character */
+
+          break;
+        }
+
+      count += bread; /* Read one more byte */
+      buf += bread;   /* Move ahead one spot in the buffer */
+    }
+
+  return count;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char **argv)
+{
+  int fd;
+  int sock;
+  int err;
+  int c;
+  ssize_t bread;
+  ssize_t bsent;
+  struct sockaddr_in client;
+
+  /* Parse command line options */
+
+  while ((c = getopt(argc, argv, ":vdn")) != -1)
+    {
+      switch (c)
+        {
+        case 'v':
+
+          /* Print version and exit */
+
+          printf("%s " SYSLOGD_VERSION " (NuttX)\n", g_syslogd);
+          return EXIT_SUCCESS;
+          break;
+
+        case 'd':
+
+          /* Enable debug mode and stay in foreground */
+
+          g_debugmode = true;
+          g_background = false;
+          break;
+
+        case 'n':
+
+          /* Stay in foreground */
+
+          g_background = false;
+          break;
+
+        case '?':
+          print_usage();
+          exit(EXIT_FAILURE);
+          break;
+        }
+    }
+
+  /* Once all arguments are parsed, fork here to run in the background.
+   * TODO: what's a good way to tell if NuttX platform has fork()?
+   */
+
+  if (g_background)
+    {
+      err = fork();

Review Comment:
   fork  doesn't work well on nuttx, so it's better to put into background from 
nsh by &



##########
system/syslogd/syslogd_main.c:
##########
@@ -0,0 +1,291 @@
+/****************************************************************************
+ * apps/system/syslogd/syslogd_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for asyslogditional 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 <nuttx/config.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <getopt.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <syslog.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Program name */
+
+#define g_syslogd CONFIG_SYSTEM_SYSLOGD_PROGNAME
+
+/* syslogd program version */
+
+#define SYSLOGD_VERSION "0.0.0"
+
+/* Minimum buffer size check */
+
+#if CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE < 480
+#error "SYSTEM_SYSLOGD_ENTRYSIZE must be more than 480 to satisfy RFC 5424"
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+bool g_debugmode = false;
+bool g_background = true;
+char g_buffer[CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: print_usage
+ ****************************************************************************/
+
+static void print_usage(void)
+{
+  fprintf(stderr, "usage:\n");
+  fprintf(stderr, "  %s [-vdn]\n", g_syslogd);
+}
+
+/****************************************************************************
+ * Name: read_log
+ *
+ * Description:
+ *   Read a single log (end indicated by a newline character) into `buf`.
+ ****************************************************************************/
+
+static ssize_t read_log(int fd, char *buf, size_t n)
+{
+  ssize_t count = 0;
+  ssize_t bread;
+
+  while (count < n)
+    {
+      bread = read(fd, buf, 1);
+
+      /* Error occurred */
+
+      if (bread < 0)
+        {
+          return bread;
+        }
+
+      /* End of stream reached */
+
+      if (bread == 0)
+        {
+          break;
+        }
+
+      /* Newline character reached */
+
+      if (*buf == '\n')
+        {
+          /* Don't increment count so user 'skips' the '\n' character */
+
+          break;
+        }
+
+      count += bread; /* Read one more byte */
+      buf += bread;   /* Move ahead one spot in the buffer */
+    }
+
+  return count;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char **argv)
+{
+  int fd;
+  int sock;
+  int err;
+  int c;
+  ssize_t bread;
+  ssize_t bsent;
+  struct sockaddr_in client;
+
+  /* Parse command line options */
+
+  while ((c = getopt(argc, argv, ":vdn")) != -1)
+    {
+      switch (c)
+        {
+        case 'v':
+
+          /* Print version and exit */
+
+          printf("%s " SYSLOGD_VERSION " (NuttX)\n", g_syslogd);
+          return EXIT_SUCCESS;
+          break;
+
+        case 'd':
+
+          /* Enable debug mode and stay in foreground */
+
+          g_debugmode = true;
+          g_background = false;
+          break;
+
+        case 'n':
+
+          /* Stay in foreground */
+
+          g_background = false;
+          break;
+
+        case '?':
+          print_usage();
+          exit(EXIT_FAILURE);
+          break;
+        }
+    }
+
+  /* Once all arguments are parsed, fork here to run in the background.
+   * TODO: what's a good way to tell if NuttX platform has fork()?
+   */
+
+  if (g_background)
+    {
+      err = fork();
+      if (err < 0)
+        {
+          fprintf(stderr, "Failed to run %s in the background: %d\n",
+                  g_syslogd, errno);
+          return EXIT_FAILURE;
+        }
+      else if (err != 0)
+        {
+          /* This is the parent process, we can quit. */
+
+          return EXIT_SUCCESS;
+        }
+    }
+
+  /* Set up client connection information */
+
+  client.sin_family = AF_INET;
+  client.sin_port = htons(CONFIG_SYSTEM_SYSLOGD_PORT);
+  client.sin_addr.s_addr = inet_addr(CONFIG_SYSTEM_SYSLOGD_ADDR);
+
+  /* Create a UDP socket */
+
+  if (g_debugmode)

Review Comment:
   change to local variable



##########
system/syslogd/syslogd_main.c:
##########
@@ -0,0 +1,291 @@
+/****************************************************************************
+ * apps/system/syslogd/syslogd_main.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for asyslogditional 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 <nuttx/config.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <getopt.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+#include <syslog.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Program name */
+
+#define g_syslogd CONFIG_SYSTEM_SYSLOGD_PROGNAME
+
+/* syslogd program version */
+
+#define SYSLOGD_VERSION "0.0.0"
+
+/* Minimum buffer size check */
+
+#if CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE < 480
+#error "SYSTEM_SYSLOGD_ENTRYSIZE must be more than 480 to satisfy RFC 5424"
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+bool g_debugmode = false;
+bool g_background = true;
+char g_buffer[CONFIG_SYSTEM_SYSLOGD_ENTRYSIZE];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: print_usage
+ ****************************************************************************/
+
+static void print_usage(void)
+{
+  fprintf(stderr, "usage:\n");
+  fprintf(stderr, "  %s [-vdn]\n", g_syslogd);
+}
+
+/****************************************************************************
+ * Name: read_log
+ *
+ * Description:
+ *   Read a single log (end indicated by a newline character) into `buf`.
+ ****************************************************************************/
+
+static ssize_t read_log(int fd, char *buf, size_t n)
+{
+  ssize_t count = 0;
+  ssize_t bread;
+
+  while (count < n)
+    {
+      bread = read(fd, buf, 1);
+
+      /* Error occurred */
+
+      if (bread < 0)
+        {
+          return bread;
+        }
+
+      /* End of stream reached */
+
+      if (bread == 0)
+        {
+          break;
+        }
+
+      /* Newline character reached */
+
+      if (*buf == '\n')
+        {
+          /* Don't increment count so user 'skips' the '\n' character */
+
+          break;
+        }
+
+      count += bread; /* Read one more byte */
+      buf += bread;   /* Move ahead one spot in the buffer */
+    }
+
+  return count;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+int main(int argc, FAR char **argv)
+{
+  int fd;
+  int sock;
+  int err;
+  int c;
+  ssize_t bread;
+  ssize_t bsent;
+  struct sockaddr_in client;
+
+  /* Parse command line options */
+
+  while ((c = getopt(argc, argv, ":vdn")) != -1)
+    {
+      switch (c)
+        {
+        case 'v':
+
+          /* Print version and exit */
+
+          printf("%s " SYSLOGD_VERSION " (NuttX)\n", g_syslogd);

Review Comment:
   use argv[0] and remove g_syslogd



-- 
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-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to