Skip to content

Commit

Permalink
convert all command handlers to variadic functions
Browse files Browse the repository at this point in the history
Resolves dozens of: warning: passing arguments to a function without a
  prototype is deprecated in all versions of C and is not supported in C2x

In traditional C the empty argument list () indicates that a function
accepts an arbitrary amount of arguments of arbitary types. A function
which takes no arguments must be declared with a (void) prototype instead.

nsh has been making elaborate use of this feature by defining command handlers
with () and passing arbitrary arguments to handlers depending on the use case.
This coding style is not future-proof. In C++, () means "no arguments", the
same as (void), and clang is now warning about this C feature going away in
C2x, aligning C with C++.

Make nsh future-proof by using variadic function prototypes (...) instead
of () prototypes. Every handler now uses (int argc, char **argv, ...) as
prototype: the argument count, an argument vector, followed by arbitrary
arguments which can be extracted using the va_arg(3) function from stdarg.h.
The variadic part of the argument list can be used to pass arguments of types
other than char * without needing to convert them to strings and back.

Using () as function prototype defeats type-checking done by the compiler,
and the same is true for variadic arguments. We still need to manually make
sure that the correct amount and types of arguments are passed in the variadic
part of the handler's argument list.

Many handlers already use the argument count + argument vector approach and
do not need internal changes, just a new prototype.
Some handlers now require use of va_start/va_arg/va_end to access additional
arguments. For example, the interfaces handlers were passing an interface
name and file descriptor before the argument count/vector. The order of
arguments is now swapped, such that interface name and file descriptor get
passed as part of the variable argument list.

Testing + OK Tom
  • Loading branch information
stspdotname committed Oct 31, 2024
1 parent c01470d commit 01bc5d4
Show file tree
Hide file tree
Showing 28 changed files with 894 additions and 427 deletions.
2 changes: 1 addition & 1 deletion arp.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ rtget(struct sockaddr_inarp **sinp, struct sockaddr_dl **sdlp,
* Set an individual arp entry
*/
int
arpset(int argc, char *argv[])
arpset(int argc, char *argv[], ...)
{
struct sockaddr_inarp *sin;
struct sockaddr_dl *sdl;
Expand Down
4 changes: 2 additions & 2 deletions bgpnsh/bgpnsh.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Menu showlist[] = {
};

static int
showcmd(int argc, char **argv)
showcmd(int argc, char **argv, ...)
{
Menu *s; /* pointer to current command */

Expand Down Expand Up @@ -106,7 +106,7 @@ showcmd(int argc, char **argv)
}

int
quit(void)
quit(int argc, char **argv, ...)
{
printf("%% Session terminated.\n");
exit(0);
Expand Down
89 changes: 79 additions & 10 deletions bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdlib.h>
#include <stdint.h>
#include <sys/param.h>
Expand Down Expand Up @@ -115,10 +116,18 @@ static struct brc brps[] = {
};

int
brport(char *ifname, int ifs, int argc, char **argv)
brport(int argc, char **argv, ...)
{
int set, i;
struct brc *x;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -222,12 +231,20 @@ static struct brc brvs[] = {
};

int
brval(char *ifname, int ifs, int argc, char **argv)
brval(int argc, char **argv, ...)
{
int set;
u_int32_t val = 0;
const char *errmsg = NULL;
struct brc *x;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -323,8 +340,17 @@ brval(char *ifname, int ifs, int argc, char **argv)
}

int
brrule(char *ifname, int ifs, int argc, char **argv)
brrule(int argc, char **argv, ...)
{
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
printf("%% all rules for a member must be applied in order\n");
printf("%% use flush bridge-rules <bridge> <member>\n");
Expand All @@ -345,9 +371,17 @@ brrule(char *ifname, int ifs, int argc, char **argv)
}

int
brstatic(char *ifname, int ifs, int argc, char **argv)
brstatic(int argc, char **argv, ...)
{
int set;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -387,11 +421,19 @@ static struct brd brds[] = {
};

int
brpri(char *ifname, int ifs, int argc, char **argv)
brpri(int argc, char **argv, ...)
{
int set, val;
const char *errmsg = NULL;
struct brd *x;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -469,10 +511,16 @@ brpri(char *ifname, int ifs, int argc, char **argv)
* flush wrappers here
*/
int
flush_bridgedyn(char *brdg)
flush_bridgedyn(int argc, char **argv, ...)
{
va_list ap;
char *brdg;
int ifs;

va_start(ap, argv);
brdg = va_arg(ap, char *);
va_end (ap);

ifs = socket(AF_INET, SOCK_DGRAM, 0);
if (ifs < 0) {
printf("%% socket: %s\n", strerror(errno));
Expand All @@ -492,10 +540,16 @@ flush_bridgedyn(char *brdg)
}

int
flush_bridgeall(char *brdg)
flush_bridgeall(int argc, char **argv, ...)
{
va_list ap;
char *brdg;
int ifs;

va_start(ap, argv);
brdg = va_arg(ap, char *);
va_end(ap);

ifs = socket(AF_INET, SOCK_DGRAM, 0);
if (ifs < 0) {
printf("%% socket: %s\n", strerror(errno));
Expand All @@ -515,10 +569,17 @@ flush_bridgeall(char *brdg)
}

int
flush_bridgerule(char *brdg, char *member)
flush_bridgerule(int argc, char **argv, ...)
{
va_list ap;
char *brdg, *member;
int ifs;

va_start(ap, argv);
brdg = va_arg(ap, char *);
member = va_arg(ap, char *);
va_end(ap);

ifs = socket(AF_INET, SOCK_DGRAM, 0);
if (ifs < 0) {
printf("%% socket: %s\n", strerror(errno));
Expand Down Expand Up @@ -1625,9 +1686,17 @@ brprotect_usage(void)
}

int
brprotect(char *ifname, int ifs, int argc, char **argv)
brprotect(int argc, char **argv, ...)
{
int set;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -1755,7 +1824,7 @@ show_bridge(char *ifname)
}

int
show_bridges(int argc, char **argv)
show_bridges(int argc, char **argv, ...)
{
switch (argc) {
case 2:
Expand Down
41 changes: 37 additions & 4 deletions carp.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
#include <errno.h>
#include <sys/limits.h>
Expand Down Expand Up @@ -46,13 +47,21 @@ static struct intc {
static const char *carp_bal_modes[] = { CARP_BAL_MODES };

int
intcarp(char *ifname, int ifs, int argc, char **argv)
intcarp(int argc, char **argv, ...)
{
const char *errmsg = NULL;
struct ifreq ifr;
struct carpreq creq;
int set, bal_mode = 0, val = 0;
struct intc *x;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -196,11 +205,19 @@ intcarp(char *ifname, int ifs, int argc, char **argv)
}

int
intcpass(char *ifname, int ifs, int argc, char **argv)
intcpass(int argc, char **argv, ...)
{
struct ifreq ifr;
struct carpreq creq;
int set;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -238,13 +255,21 @@ intcpass(char *ifname, int ifs, int argc, char **argv)
}

int
intcnode(char *ifname, int ifs, int argc, char **argv)
intcnode(int argc, char **argv, ...)
{
struct ifreq ifr;
struct carpreq creq;
const char *errmsg = NULL;
int set, i, last;
u_int vhid, advskew;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down Expand Up @@ -428,11 +453,19 @@ carp_state(int s, char *ifname, FILE *outfile)
}

int
intcdev(char *ifname, int ifs, int argc, char **argv)
intcdev(int argc, char **argv, ...)
{
struct ifreq ifr;
struct carpreq creq;
int set;
va_list ap;
char *ifname;
int ifs;

va_start(ap, argv);
ifname = va_arg(ap, char *);
ifs = va_arg(ap, int);
va_end(ap);

if (NO_ARG(argv[0])) {
set = 0;
Expand Down
Loading

1 comment on commit 01bc5d4

@smytht
Copy link
Collaborator

@smytht smytht commented on 01bc5d4 Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks or that

Please sign in to comment.