Changeset View
Standalone View
sbin/ping6/options.c
- This file was added.
/*- | |||||
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD | |||||
* | |||||
* Copyright (C) 2019 Jan Sucan <jansucan@FreeBSD.org> | |||||
* All rights reserved. | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
*/ | |||||
/* $KAME: ping6.c,v 1.169 2003/07/25 06:01:47 itojun Exp $ */ | |||||
/*- | |||||
* SPDX-License-Identifier: BSD-3-Clause | |||||
* | |||||
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project. | |||||
* All rights reserved. | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* 3. Neither the name of the project nor the names of its contributors | |||||
* may be used to endorse or promote products derived from this software | |||||
* without specific prior written permission. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE PROJECT AND CONTRIBUTORS ``AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE PROJECT OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
*/ | |||||
/* BSDI ping.c,v 2.3 1996/01/21 17:56:50 jch Exp */ | |||||
/* | |||||
* Copyright (c) 1989, 1993 | |||||
* The Regents of the University of California. All rights reserved. | |||||
* | |||||
* This code is derived from software contributed to Berkeley by | |||||
* Mike Muuss. | |||||
* | |||||
* Redistribution and use in source and binary forms, with or without | |||||
* modification, are permitted provided that the following conditions | |||||
* are met: | |||||
* 1. Redistributions of source code must retain the above copyright | |||||
* notice, this list of conditions and the following disclaimer. | |||||
* 2. Redistributions in binary form must reproduce the above copyright | |||||
* notice, this list of conditions and the following disclaimer in the | |||||
* documentation and/or other materials provided with the distribution. | |||||
* 3. Neither the name of the University nor the names of its contributors | |||||
* may be used to endorse or promote products derived from this software | |||||
* without specific prior written permission. | |||||
* | |||||
* THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND | |||||
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE | |||||
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE | |||||
* ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE | |||||
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL | |||||
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS | |||||
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | |||||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT | |||||
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY | |||||
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF | |||||
* SUCH DAMAGE. | |||||
*/ | |||||
#if 0 | |||||
#ifndef lint | |||||
static const char copyright[] = | |||||
"@(#) Copyright (c) 1989, 1993\n\ | |||||
The Regents of the University of California. All rights reserved.\n"; | |||||
#endif /* not lint */ | |||||
#ifndef lint | |||||
static char sccsid[] = "@(#)ping.c 8.1 (Berkeley) 6/5/93"; | |||||
#endif /* not lint */ | |||||
#endif | |||||
#include <sys/cdefs.h> | |||||
__FBSDID("$FreeBSD$"); | |||||
#include <string.h> | |||||
#include <unistd.h> | |||||
#include "options.h" | |||||
#ifndef IPSEC | |||||
#define ADDOPTS | |||||
#else | |||||
#ifdef IPSEC_POLICY_IPSEC | |||||
#define ADDOPTS "P:" | |||||
#else | |||||
#define ADDOPTS "ZE" | |||||
#endif /*IPSEC_POLICY_IPSEC*/ | |||||
#endif | |||||
bool | |||||
options_parse(int argc, char *argv[], | |||||
struct options_processed *const opts, u_int *const flags) | |||||
{ | |||||
int ch; | |||||
memset(opts, 0, sizeof(*opts)); | |||||
while ((ch = getopt(argc, argv, | |||||
"k:b:c:DdfHe:m:I:i:l:unNop:qaAS:s:OvyYW:t:" ADDOPTS)) != -1) { | |||||
switch (ch) { | |||||
case 'k': | |||||
*flags &= ~F_NOUSERDATA; | |||||
*flags |= F_NODEADDR; | |||||
opts->arg_addrtype = optarg; | |||||
break; | |||||
case 'b': | |||||
#if defined(SO_SNDBUF) && defined(SO_RCVBUF) | |||||
opts->arg_sock_buff_size = optarg; | |||||
break; | |||||
#else | |||||
errx(1, "-b option ignored: SO_SNDBUF/SO_RCVBUF socket" | |||||
"options not supported"); | |||||
/*NOTREACHED*/ | |||||
#endif | |||||
case 'c': | |||||
opts->arg_packet_count = optarg; | |||||
break; | |||||
case 'D': | |||||
*flags |= F_DONTFRAG; | |||||
break; | |||||
case 'd': | |||||
*flags |= F_SO_DEBUG; | |||||
asomers: Do you really save much code by handling optarg down here? This obvious method, handling… | |||||
Done Inline ActionsDone. jansucan: Done. | |||||
Not Done Inline ActionsUltimately there will be two different consumers of options_parse, right? If so, then there's going to be duplicate code for stuff like string->int conversion. I think it would be a good idea to move that stuff into options_parse. asomers: Ultimately there will be two different consumers of `options_parse`, right? If so, then… | |||||
Done Inline ActionsI agree (I suppose that the two consumers are ping and ping6). I would like to continue sending the changes from the unified option parsing in my branch at GitHub for a review. It contains code for conversion of the arguments. Could it be done in this way? jansucan: I agree (I suppose that the two consumers are ping and ping6). I would like to continue sending… | |||||
Not Done Inline ActionsI don't understand. Could what be done in what way? asomers: I don't understand. Could what be done in what way? | |||||
Done Inline ActionsTo me it seemed that you would like this diff to contain the string->int conversion. I think it would be better to implement it in a separate diff (the changes are ready in my branch on GitHub) because this one is a dependency the option compatibility getopt loop which will not cause duplication of the conversion code. jansucan: To me it seemed that you would like this diff to contain the string->int conversion. I think it… | |||||
Not Done Inline ActionsAre you trying to see that the natural order for patches would be:
asomers: Are you trying to see that the natural order for patches would be:
1) This one
2) Backwards… | |||||
Done Inline ActionsSomething like that.
I have a question concerning step 4. This diff uses struct options_processed. Maybe more appropriate name would be 'struct options_to_process' or something like that. This structure will be private for the option parsing code. It is used to carry information between the two stages of option parsing: detecting options with their arguments, and processing those arguments. The struct options will be used for holding the parsed arguments and will be use outside the option parsing code, in ping and ping6 code. I will be glad to use some better naming, if you give me some suggestions. :-) jansucan: Something like that.
1. This diff.
2. Merge ping and ping6. Because if we applied the patch… | |||||
Not Done Inline ActionsRegarding step 4, I still don't see a compelling reason to separate string->integer conversion from getopt. I would just combine them, and call the structure struct options. asomers: Regarding step 4, I still don't see a compelling reason to separate string->integer conversion… | |||||
Done Inline ActionsShould checking for conversion errors be a part of the getopt loop too? jansucan: Should checking for conversion errors be a part of the getopt loop too? | |||||
Not Done Inline ActionsYou mean like the kind of error you would get from ping -c not_an_integer? Yes, I think so. asomers: You mean like the kind of error you would get from `ping -c not_an_integer`? Yes, I think so. | |||||
Done Inline ActionsOK. Syntactic checks will be in getopt loop, semantic checks will remain in main(). jansucan: OK. Syntactic checks will be in getopt loop, semantic checks will remain in `main()`. | |||||
Done Inline ActionsI did it only for -b option as a preview. Could you please check it? jansucan: I did it only for `-b` option as a preview. Could you please check it? | |||||
break; | |||||
case 'f': | |||||
*flags |= F_FLOOD; | |||||
break; | |||||
case 'e': | |||||
opts->arg_gateway = optarg; | |||||
Not Done Inline ActionsHere's another wart. ping allows the -c argument to be in any base, but ping6 forces base 10. It would be good to fix that in a separate commit. asomers: Here's another wart. ping allows the `-c` argument to be in any base, but ping6 forces base 10. | |||||
Done Inline ActionsI will fix it in the next diff. jansucan: I will fix it in the next diff. | |||||
break; | |||||
case 'H': | |||||
*flags |= F_HOSTNAME; | |||||
break; | |||||
case 'm': | |||||
opts->arg_hoplimit = optarg; | |||||
break; | |||||
case 'I': | |||||
*flags |= F_INTERFACE; | |||||
opts->arg_interface = optarg; | |||||
(opts->count_interface)++; | |||||
break; | |||||
case 'i': | |||||
*flags |= F_INTERVAL; | |||||
opts->arg_interval = optarg; | |||||
break; | |||||
case 'l': | |||||
opts->arg_preload = optarg; | |||||
break; | |||||
case 'u': | |||||
#ifdef IPV6_USE_MIN_MTU | |||||
(opts->count_use_min_mtu)++; | |||||
break; | |||||
#else | |||||
errx(1, "-u is not supported on this platform"); | |||||
/*NOTREACHED*/ | |||||
#endif | |||||
case 'n': | |||||
*flags &= ~F_HOSTNAME; | |||||
break; | |||||
case 'N': | |||||
*flags |= F_NIGROUP; | |||||
(opts->count_nigroup)++; | |||||
break; | |||||
case 'o': | |||||
*flags |= F_ONCE; | |||||
break; | |||||
case 'p': | |||||
*flags |= F_PINGFILLED; | |||||
opts->arg_pattern = optarg; | |||||
break; | |||||
case 'q': | |||||
*flags |= F_QUIET; | |||||
break; | |||||
case 'a': | |||||
*flags |= F_AUDIBLE; | |||||
break; | |||||
case 'A': | |||||
*flags |= F_MISSED; | |||||
break; | |||||
case 'S': | |||||
*flags |= F_SRCADDR; | |||||
opts->arg_source_address = optarg; | |||||
break; | |||||
case 's': | |||||
opts->arg_packet_size = optarg; | |||||
break; | |||||
case 'O': | |||||
*flags &= ~F_NOUSERDATA; | |||||
*flags |= F_SUPTYPES; | |||||
break; | |||||
case 'v': | |||||
*flags |= F_VERBOSE; | |||||
break; | |||||
case 'y': | |||||
*flags &= ~F_NOUSERDATA; | |||||
*flags |= F_FQDN; | |||||
break; | |||||
case 'Y': | |||||
*flags &= ~F_NOUSERDATA; | |||||
*flags |= F_FQDNOLD; | |||||
break; | |||||
case 'W': | |||||
*flags |= F_WAITTIME; | |||||
opts->arg_wait_time = optarg; | |||||
break; | |||||
case 't': | |||||
opts->arg_timeout = optarg; | |||||
break; | |||||
#ifdef IPSEC | |||||
#ifdef IPSEC_POLICY_IPSEC | |||||
case 'P': | |||||
*flags |= F_POLICY; | |||||
opts->arg_ipsec_policy = optarg; | |||||
break; | |||||
#else | |||||
case 'Z': | |||||
*flags |= F_AUTHHDR; | |||||
break; | |||||
case 'E': | |||||
*flags |= F_ENCRYPT; | |||||
break; | |||||
#endif /*IPSEC_POLICY_IPSEC*/ | |||||
#endif /*IPSEC*/ | |||||
default: | |||||
return (false); | |||||
} | |||||
} | |||||
return (true); | |||||
} |
Do you really save much code by handling optarg down here? This obvious method, handling optarg in each case statement, might be more verbose, but I think it would make the code in ping6.c clearer. It would allow the struct options_processed to have a more obvious and self-documenting structure, using appropriate data types for each option.