[PATCH] portmap: new applet

Marek Polacek mpolacek at redhat.com
Sun May 1 17:38:27 UTC 2011


On 05/01/2011 07:12 PM, Lukas Huba wrote:
>  include/applets.src.h |    1 +
>  include/usage.src.h   |    8 ++
>  networking/Config.src |   16 +++
>  networking/Kbuild.src |    1 +
>  networking/portmap.c  |  247 +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 273 insertions(+), 0 deletions(-)
>  create mode 100644 networking/portmap.c

You are still touching 5 files.  Only portmap.c should be needed.

> +//config: config PORTMAP
> +//config: 	bool "portmap"
> +//config: 	default n
> +//config: 	select PLATFORM_LINUX
> +//config: 	select FEATURE_SYSLOG
> +//config: 	help
> +//config: 	  RPC program, version to DARPA port mapper.
> +//config: 
> +//config: config PORTMAP_ITEMS_MAX
> +//config: 	int "Maximum RPC services"
> +//config: 	default 32
> +//config: 	depends on PORTMAP
> +//config: 	help
> +//config: 	  Maximum RPC services which portmap is able to store.
> +//config: 	  That's for better security.

After `//config:' there should be a tab.  You have space AND tab.  Please remove
the space.

> +/* portmap items */
> +static int pn;
> +static struct pmaplist *pl;
> +
> +/* default count of network interfaces (later it updates itself)
> + * it's for higher efficiency (re|m)alloc */
> +static int ifs = 5;

All globals should go to the struct globals.  See, for example, coreutils/du.c.

> +	if (pp == NULL)
> +		n = &pl;
> +	else
> +		n = &pp->pml_next;

BTW, I'd probably use ?: here, but I'm not sure what Denys likes more ;).

> +	for (p=pl; p!=NULL; p=p->pml_next) {

Missing spaces.

> +	for (p=pl; p!=NULL; p=p->pml_next) {

Again, spaces.

> +		if (p->pml_map.pm_prog == pr->pm_prog &&
> +			p->pml_map.pm_vers == pr->pm_vers) {
> +			return p->pml_map.pm_port;
> +		}


More like:
		if (p->pml_map.pm_prog == pr->pm_prog
		    && p->pml_map.pm_vers == pr->pm_vers
		) {
			return p->pml_map.pm_port;
		}


> +	switch(rqstp->rq_proc) {

Space after `switch'.

> +int portmap_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> +int portmap_main(int argc, char **argv)
> +{
> +	char *host = (char *)"0.0.0.0";

This doesn't seem right.  I think the `host' should be const char *, since
the `create_and_bind_{stream,dgram}_or_die' expect const char *.

> +	if (argc >= 2)

Is this `if' really needed?

> +		if ((xprt = svcudp_create(sock)) == (SVCXPRT *)NULL)
> +			bb_error_msg_and_die("cannot start portmap");

Please no assignments in `if'.

	Marek


More information about the busybox mailing list