mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Alexander Aring <aar@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH RFC] common: env: handle var="" not as var=NULL
Date: Tue, 14 Jun 2016 08:25:06 +0200	[thread overview]
Message-ID: <20160614062506.GO31666@pengutronix.de> (raw)
In-Reply-To: <20160613105152.28770-1-aar@pengutronix.de>

On Mon, Jun 13, 2016 at 12:51:52PM +0200, Alexander Aring wrote:
> This patch changes the behaviour which I think is used everywhere in
> barebox code. Currently var="" and var=NULL will end in the same
> behaviour.
> 
> New behaviour is:
> 
> setenv(name, "")	-> sets name to ""
> \-> getenv(name)	-> ""
> 
> setenv(name, NULL)	-> deletes env
> \-> getenv(name)	-> NULL
> 
> It seems that barebox code already wants such behaviour, by run
> 
> $ grep -r "\"\"" | grep setenv
> 
> it finds most all related code sections which use "".
> 
> I detected the issue while commenting #ip="foo" in my env. The code
> part:
> 
> net/ifup.c:	setenv("ip", "");
> 
> Sets at first "ip" to "" but will end in the old behaviour as "delete".
> The getenv("ip") in ifup.c will not check on the NULL value and has a
> general handling for unknown ip env.
> 
> Note:
> Also check commit e7ed69d5 ("setenv: be robust against setenv(<var>,
> NULL") which says that hush will handle "" and NULL as the same.
> 
> Another check would be:
> 
> $ grep -r "NULL" | grep setenv
> 
> which should show the behaviour in case of deleting env.
> 
> Signed-off-by: Alexander Aring <aar@pengutronix.de>
> ---
> I test the ip env stuff only that barebox will not crash anymore but I
> think it will break some other setenv/getenv behaviour.
> 
> The question is "Shall we keep that "" and NULL ends in same behaviour"
> and fix the code which use it wrong, e.g. setenv("ip", ""); getenv("ip").

I'm not sure at the moment if a var="" should delete a variable or just
set it to an empty string. In any case we should fix net/ifup.c.
getenv() can return NULL and the code should be prepared for that.

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:[~2016-06-14  6:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-13 10:51 Alexander Aring
2016-06-14  6:25 ` Sascha Hauer [this message]

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=20160614062506.GO31666@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=aar@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox