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/07/20 13:25:49 UTC

[GitHub] [incubator-nuttx] ALTracer commented on pull request #6640: net/slip: Rename and clarify orphaned Kconfig options

ALTracer commented on PR #6640:
URL: https://github.com/apache/incubator-nuttx/pull/6640#issuecomment-1190285198

   > Seems like `TUN_NINTERFACES` also should be renamed to `NET_TUN_NINTERFACES` based on this strategy or we should get back from `NET_SLIP_NINTERFACES` to `SLIP_NINTERFACES`
   
   @pkarashchenko That corresponds to drivers/net/tun.c and isn't broken like SLIP was. Do you want the rename just for consistency? Nothing else using the NINTERFACES keys uses the NET_ prefix apart from NET_SLIP and NET_TUN. 
   Only ENC28J60 and ENCX24J600 (has a copy-paste error in Kconfig) are actual external NICs with SPI interface, DM9X uses some MMIO on ntosd-dm320:nsh.
   ```sh
   $ find . -name Kconfig | xargs -n1 grep -l NINTERFACES
   ./drivers/lcd/Kconfig
   ./drivers/net/Kconfig
   ./drivers/wireless/ieee80211/bcm43xxx/Kconfig
   ./net/Kconfig
   $ find . -name Kconfig | xargs -n1 grep NINTERFACES
   config P14201_NINTERFACES
   config UG9664HSWAG01_NINTERFACES
   config SSD1351_NINTERFACES
   config PCD8544_NINTERFACES
   config ST7565_NINTERFACES
   config ST7567_NINTERFACES
   config UG2864AMBAG01_NINTERFACES
   config MEMLCD_NINTERFACES
   config LCD_ILI9340_NINTERFACES
           depends on LCD_ILI9340_NINTERFACES = 1 || LCD_ILI9340_NINTERFACES = 2
           depends on LCD_ILI9340_NINTERFACES = 2
   config LCD_ILI9341_NINTERFACES
           depends on LCD_ILI9341_NINTERFACES = 1 || LCD_ILI9341_NINTERFACES = 2
           depends on LCD_ILI9341_NINTERFACES = 2
   config DM9X_NINTERFACES
   config ENC28J60_NINTERFACES
   config ENC28J60_NINTERFACES
   config IEEE80211_BROADCOM_NINTERFACES
   config NET_SLIP_NINTERFACES
   config NET_TUN_NINTERFACES
   ```
   CONFIG_TUN_NINTERFACES is used in C source code. I could make a patch replacing these with CONFIG_NET_TUN_INTERFACES:
   ```diff
   diff --git a/drivers/net/tun.c b/drivers/net/tun.c
   index 61ad6696d0..2bae04620e 100644
   --- a/drivers/net/tun.c
   +++ b/drivers/net/tun.c
   @@ -91,12 +91,12 @@
    
    #define TUNWORK LPWORK
    
   -/* CONFIG_TUN_NINTERFACES determines the number of physical interfaces
   +/* CONFIG_NET_TUN_NINTERFACES determines the number of physical interfaces
     * that will be supported.
     */
    
   -#ifndef CONFIG_TUN_NINTERFACES
   -#  define CONFIG_TUN_NINTERFACES 1
   +#ifndef CONFIG_NET_TUN_NINTERFACES
   +#  define CONFIG_NET_TUN_NINTERFACES 1
    #endif
    
    /* Make sure that packet buffers include in configured guard size and are an
   @@ -227,7 +227,7 @@ static int tun_poll(FAR struct file *filep, FAR struct pollfd *fds,
     ****************************************************************************/
    
    static struct tun_driver_s g_tun;
   -static struct tun_device_s g_tun_devices[CONFIG_TUN_NINTERFACES];
   +static struct tun_device_s g_tun_devices[CONFIG_NET_TUN_NINTERFACES];
    
    static const struct file_operations g_tun_file_ops =
    {
   @@ -1442,7 +1442,7 @@ static int tun_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
            }
    
          for (intf = 0;
   -           intf < CONFIG_TUN_NINTERFACES && !(free_tuns & 1);
   +           intf < CONFIG_NET_TUN_NINTERFACES && !(free_tuns & 1);
               intf++, free_tuns >>= 1);
    
          ret = tun_dev_init(&g_tun_devices[intf], filep,
   @@ -1489,7 +1489,7 @@ int tun_initialize(void)
    {
      nxsem_init(&g_tun.waitsem, 0, 1);
    
   -  g_tun.free_tuns = (1 << CONFIG_TUN_NINTERFACES) - 1;
   +  g_tun.free_tuns = (1 << CONFIG_NET_TUN_NINTERFACES) - 1;
    
      register_driver("/dev/tun", &g_tun_file_ops, 0644, &g_tun);
      return OK;
   diff --git a/net/Kconfig b/net/Kconfig
   index 7288e5f3cc..ae90a9a1b0 100644
   --- a/net/Kconfig
   +++ b/net/Kconfig
   @@ -229,7 +229,7 @@ menuconfig NET_TUN
    
    if NET_TUN
    
   -config TUN_NINTERFACES
   +config NET_TUN_NINTERFACES
           int "Number of TUN interfaces"
           default 1
           range 1 8
   ```
   Or I could rename NET_SLIP_NINTERFACES back to SLIP_NINTERFACES:
   ```diff
   diff --git a/drivers/net/slip.c b/drivers/net/slip.c
   index a4c4608929..de033a4396 100644
   --- a/drivers/net/slip.c
   +++ b/drivers/net/slip.c
   @@ -88,12 +88,12 @@
    #  error "CONFIG_NET_SLIP_PKTSIZE >= 296 is required"
    #endif
    
   -/* CONFIG_NET_SLIP_NINTERFACES determines the number of physical interfaces
   +/* CONFIG_SLIP_NINTERFACES determines the number of physical interfaces
     * that will be supported.
     */
    
   -#ifndef CONFIG_NET_SLIP_NINTERFACES
   -# define CONFIG_NET_SLIP_NINTERFACES 1
   +#ifndef CONFIG_SLIP_NINTERFACES
   +# define CONFIG_SLIP_NINTERFACES 1
    #endif
    
    /*  SLIP special character codes ********************************************/
   @@ -143,11 +143,11 @@ struct slip_driver_s
     * Private Data
     ****************************************************************************/
    
   -/* We really should get rid of CONFIG_NET_SLIP_NINTERFACES and, instead,
   +/* We really should get rid of CONFIG_SLIP_NINTERFACES and, instead,
     * kmm_malloc() new interface instances as needed.
     */
    
   -static struct slip_driver_s g_slip[CONFIG_NET_SLIP_NINTERFACES];
   +static struct slip_driver_s g_slip[CONFIG_SLIP_NINTERFACES];
    
    /****************************************************************************
     * Private Function Prototypes
   @@ -445,7 +445,7 @@ static int slip_txtask(int argc, FAR char *argv[])
      int ret;
    
      nerr("index: %d\n", index);
   -  DEBUGASSERT(index < CONFIG_NET_SLIP_NINTERFACES);
   +  DEBUGASSERT(index < CONFIG_SLIP_NINTERFACES);
    
      /* Get our private data structure instance and wake up the waiting
       * initialization logic.
   @@ -667,7 +667,7 @@ static int slip_rxtask(int argc, FAR char *argv[])
      int ch;
    
      nerr("index: %d\n", index);
   -  DEBUGASSERT(index < CONFIG_NET_SLIP_NINTERFACES);
   +  DEBUGASSERT(index < CONFIG_SLIP_NINTERFACES);
    
      /* Get our private data structure instance and wake up the waiting
       * initialization logic.
   @@ -978,7 +978,7 @@ int slip_initialize(int intf, FAR const char *devname)
    
      /* Get the interface structure associated with this interface number. */
    
   -  DEBUGASSERT(intf < CONFIG_NET_SLIP_NINTERFACES);
   +  DEBUGASSERT(intf < CONFIG_SLIP_NINTERFACES);
      priv = &g_slip[intf];
    
      /* Initialize the driver structure */
   diff --git a/net/Kconfig b/net/Kconfig
   index 7288e5f3cc..af38419b15 100644
   --- a/net/Kconfig
   +++ b/net/Kconfig
   @@ -196,7 +196,7 @@ menuconfig NET_SLIP
    
    if NET_SLIP
    
   -config NET_SLIP_NINTERFACES
   +config SLIP_NINTERFACES
           int "Number of SLIP interfaces"
           default 1
           ---help---
   ```
   @xiaoxiang781216 Which of the two patch variants do you like better?
   _I wish GitHub Markdown dialect supported code spoilers._


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