All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 1/8] net: dhcp: factorise option recption handling
Date: Mon, 2 Apr 2012 21:02:46 +0200	[thread overview]
Message-ID: <20120402190246.GE3852@pengutronix.de> (raw)
In-Reply-To: <1333376350-19506-1-git-send-email-plagnioj@jcrosoft.com>

On Mon, Apr 02, 2012 at 04:19:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Instead of using a static switch case the handle the received option
> use an array of supported option.
> 
> This will allow to unset the env var without duplicating the code.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Much better, thanks.

> +struct dhcp_opt {
> +	unsigned char option;
> +	const char *barebox_var_name;
> +	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
> +	void *data;
> +
> +	/* request automatically the option when creating the DHCP request */
> +	bool optinal;

s/optinal/optional/?

You can optimize the size of this struct a bit by putting this bool
after the unsigned char above.

> +
> +	struct bootp *bp;
> +};
> +
> +static void netmask_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> +{
> +	IPaddr_t ip;
> +
> +	ip = net_read_ip(popt);
> +	net_set_netmask(ip);
> +}
> +
> +static void gateway_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> +{
> +	IPaddr_t ip;
> +
> +	ip = net_read_ip(popt);
> +	net_set_gateway(ip);
> +}
> +
> +static void env_ip_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> +{
> +	IPaddr_t ip;
> +
> +	if (!opt->barebox_var_name) {
> +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> +		       __func__, opt->option);
> +		return;
> +	}

I think we don't need this check. It is easy enough to review the code
to not have this bug.

> +
> +	ip = net_read_ip(popt);
> +	setenv_ip(opt->barebox_var_name, ip);
> +}
> +
> +static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> +{
> +	char str[256];
> +
> +	if (!opt->barebox_var_name) {
> +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> +		       __func__, opt->option);
> +		return;
> +	}

ditto

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2012-04-02 19:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 19:02   ` Sascha Hauer [this message]
2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
2012-04-03  6:57       ` Sascha Hauer
2012-04-03  4:57     ` [PATCH 1/8 v4] " Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 2/8] net: dhcp: reset env variable before do a dhcp request Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 3/8] net: dhcp: add support of tftp name server Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 4/8] net: dhcp: factorise setting option code Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 5/8] net: dhcp: allow to set transmitted client id Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 6/8] net: dhcp: allow to set transmitted client uuid Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 7/8] net: dhcp: allow to set transmitted user class Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 8/8] net: env: getenv_ip use resolv Jean-Christophe PLAGNIOL-VILLARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120402190246.GE3852@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=plagnioj@jcrosoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.