matiamic commented on code in PR #3181: URL: https://github.com/apache/nuttx-apps/pull/3181#discussion_r2353224891
########## netutils/plcatool/plcatool.c: ########## @@ -0,0 +1,717 @@ +/**************************************************************************** + * apps/netutils/plcatool/plcatool.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 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 <nuttx/config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <string.h> +#include <errno.h> + +#include <net/if.h> +#include <sys/ioctl.h> + +#include "oa_tc14.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define ARGS_REMAIN(argc, pos) ((argc) - (pos)) + +#define PLCATOOL_CFG_SET_BIT 0x8000 +#define PLCATOOL_CFG_SET(cfg, field, val) \ + do {(cfg)->field = PLCATOOL_CFG_SET_BIT | ((val) & 0xff);} while (0) + +#define PLCATOOL_CFG_VAL(cfg, field) ((cfg)->field & 0xff) + +#define NODE_ID_MIN 0 +#define NODE_ID_MAX 255 + +#define NODE_CNT_MIN 1 +#define NODE_CNT_MAX 255 + +#define TO_TMR_MIN 0 +#define TO_TMR_MAX 255 + +#define BURST_CNT_MIN 0 +#define BURST_CNT_MAX 255 + +#define BURST_TMR_MIN 0 +#define BURST_TMR_MAX 255 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +enum plcatool_cmd_e +{ + PLCA_CMD_SET, + PLCA_CMD_GET, + PLCA_CMD_STATUS, + PLCA_CMD_HELP +}; + +/* Lower 8 bits are value, the most significant bit indicates whether set */ + +struct plcatool_cfg_s +{ + enum plcatool_cmd_e cmd; + + FAR char *ifname; + + uint8_t phy; + + uint16_t enable; + uint16_t node_cnt; + uint16_t node_id; + uint16_t to_tmr; + uint16_t burst_cnt; + uint16_t burst_tmr; +}; + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int get_num(FAR const char *str, FAR int *result); +static int parse_args(int argc, FAR char *argv[], + FAR struct plcatool_cfg_s *cfg); +static int write_plca_mmd(FAR const char *ifname, int phy, + uint16_t address, uint16_t data); +static int read_plca_mmd(FAR const char *ifname, int phy, + uint16_t address, FAR uint16_t *data); +static int verify_support(FAR struct plcatool_cfg_s *cfg, + FAR bool *supported); +static int plcatool_set(FAR struct plcatool_cfg_s *cfg); +static int plcatool_get(FAR struct plcatool_cfg_s *cfg); +static int plcatool_status(FAR struct plcatool_cfg_s *cfg); +static void plcatool_usage(bool err); + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static int get_num(FAR const char *str, FAR int *result) +{ + FAR char *endptr; + *result = (int) strtol(str, &endptr, 0); + if (errno) Review Comment: I think (and based on tests) the endptr condition does not catch when the number string is out of range of the output type. In that case `strtol` returns either `LONG_MAX` or `LONG_MIN`. This changes the number the user probably thinks was entered. But I agree in this case it is redundant because the number is later checked for much smaller range. Also casting to `(int)` can have the same effect and is not cared for in any 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org