[U-Boot] [PATCH 1/3] fw_env: fix type of len

[U-Boot] [PATCH 1/3] fw_env: fix type of len

[Mike Frysinger]

This variable is assigned by a size_t, and is printed that way, but is
incorrectly declared as an int.  Which means we get warnings:
fw_env.c: In function 'fw_setenv':
fw_env.c:409:5: warning: format '%zu' expects argument of type 'size_t',
	but argument 3 has type 'int' [-Wformat]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/env/fw_env.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 9b023e8..02f97c0 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -379,7 +379,8 @@ int fw_env_write(char *name, char *value)
  */
 int fw_setenv(int argc, char *argv[])
 {
-	int i, len;
+	int i;
+	size_t len;
 	char *name;
 	char *value = NULL;
 
-- 
1.7.12.4

[U-Boot] [PATCH 2/3] fw_env: fix incorrect usage of open(O_CREAT)

[Mike Frysinger]

When using open(), the O_CREAT flag must be given a mode, otherwise it
uses random garbage from the stack.  Also, it can fail to build:

In file included from /usr/include/fcntl.h:290:0,
                 from fw_env_main.c:42:
In function 'open',
    inlined from 'main' at fw_env_main.c:97:9:
/usr/include/bits/fcntl2.h:50:24: error: call to '__open_missing_mode' declared
	with attribute error: open with O_CREAT in second argument needs 3 arguments

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/env/fw_env_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
index c855f4c..40ea3f6 100644
--- a/tools/env/fw_env_main.c
+++ b/tools/env/fw_env_main.c
@@ -94,7 +94,7 @@ int main(int argc, char *argv[])
 	int lockfd = -1;
 	int retval = EXIT_SUCCESS;
 
-	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC);
+	lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
 	if (-1 == lockfd) {
 		fprintf(stderr, "Error opening lock file %s\n", lockname);
 		return EXIT_FAILURE;
-- 
1.7.12.4

[U-Boot] [PATCH 2/3] fw_env: fix incorrect usage of open(O_CREAT)

[Christian Riesch]

Hi,

On Sun, Nov 11, 2012 at 6:47 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> When using open(), the O_CREAT flag must be given a mode, otherwise it
> uses random garbage from the stack.  Also, it can fail to build:
>
> In file included from /usr/include/fcntl.h:290:0,
>                  from fw_env_main.c:42:
> In function 'open',
>     inlined from 'main' at fw_env_main.c:97:9:
> /usr/include/bits/fcntl2.h:50:24: error: call to '__open_missing_mode' declared
>         with attribute error: open with O_CREAT in second argument needs 3 arguments

Could someone please pick up this patchset?

http://patchwork.ozlabs.org/patch/198241/
http://patchwork.ozlabs.org/patch/198243/
http://patchwork.ozlabs.org/patch/198242/

make env is currently broken for me.
Thank you!
Regards, Christian

>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  tools/env/fw_env_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/env/fw_env_main.c b/tools/env/fw_env_main.c
> index c855f4c..40ea3f6 100644
> --- a/tools/env/fw_env_main.c
> +++ b/tools/env/fw_env_main.c
> @@ -94,7 +94,7 @@ int main(int argc, char *argv[])
>         int lockfd = -1;
>         int retval = EXIT_SUCCESS;
>
> -       lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC);
> +       lockfd = open(lockname, O_WRONLY | O_CREAT | O_TRUNC, 0666);
>         if (-1 == lockfd) {
>                 fprintf(stderr, "Error opening lock file %s\n", lockname);
>                 return EXIT_FAILURE;
> --
> 1.7.12.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/3] fw_env: fix incorrect usage of open(O_CREAT)

[Joe Hershberger]

Hi Mike,

On Sat, Nov 10, 2012 at 11:47 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> When using open(), the O_CREAT flag must be given a mode, otherwise it
> uses random garbage from the stack.  Also, it can fail to build:
>
> In file included from /usr/include/fcntl.h:290:0,
>                  from fw_env_main.c:42:
> In function 'open',
>     inlined from 'main' at fw_env_main.c:97:9:
> /usr/include/bits/fcntl2.h:50:24: error: call to '__open_missing_mode' declared
>         with attribute error: open with O_CREAT in second argument needs 3 arguments
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tools/env/Makefile | 11 ++++++++++-
 tools/env/fw_env.h | 25 -------------------------
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/tools/env/Makefile b/tools/env/Makefile
index ab73c8c..62a113a 100644
--- a/tools/env/Makefile
+++ b/tools/env/Makefile
@@ -24,7 +24,7 @@
 include $(TOPDIR)/config.mk
 
 HOSTSRCS := $(SRCTREE)/lib/crc32.c  fw_env.c  fw_env_main.c
-HEADERS	:= fw_env.h $(OBJTREE)/include/config.h
+HEADERS	:= fw_env.h
 
 # Compile for a hosted environment on the target
 HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
@@ -33,6 +33,15 @@ HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
 		-DUSE_HOSTCC \
 		-DTEXT_BASE=$(TEXT_BASE)
 
+# Pass CONFIG_xxx settings via the command line so that we can build w/out
+# a config.h file existing in the first place.  Useful for generic builds.
+CONFIG_VARS_TO_PASS = \
+	ENV_OVERWRITE \
+	OVERWRITE_ETHADDR_ONCE \
+	ETHADDR
+HOSTCPPFLAGS += \
+	$(foreach x,$(CONFIG_VARS_TO_PASS),$(if $(CONFIG_$(x)),-DCONFIG_$(x)=$(CONFIG_$(x))))
+
 ifeq ($(MTD_VERSION),old)
 HOSTCPPFLAGS += -DMTD_OLD
 endif
diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index a1a6807..19703c7 100644
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -21,15 +21,6 @@
  * MA 02111-1307 USA
  */
 
-/* Pull in the current config to define the default environment */
-#ifndef __ASSEMBLY__
-#define __ASSEMBLY__ /* get only #defines from config.h */
-#include <config.h>
-#undef	__ASSEMBLY__
-#else
-#include <config.h>
-#endif
-
 /*
  * To build the utility with the static configuration
  * comment out the next line.
@@ -52,22 +43,6 @@
 #define DEVICE2_ENVSECTORS     2
 #endif
 
-#ifndef CONFIG_BAUDRATE
-#define CONFIG_BAUDRATE		115200
-#endif
-
-#ifndef CONFIG_BOOTDELAY
-#define CONFIG_BOOTDELAY	5	/* autoboot after 5 seconds	*/
-#endif
-
-#ifndef CONFIG_BOOTCOMMAND
-#define CONFIG_BOOTCOMMAND							\
-	"bootp; "								\
-	"setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "	\
-	"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "	\
-	"bootm"
-#endif
-
 extern int   fw_printenv(int argc, char *argv[]);
 extern char *fw_getenv  (char *name);
 extern int fw_setenv  (int argc, char *argv[]);
-- 
1.7.12.4

[U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h

[Peter Korsgaard]

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

 Mike> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

I haven't tested this, but we have been carrying a similar patch in
buildroot for a while.

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h

[Joe Hershberger]

Hi Mike

On Sat, Nov 10, 2012 at 11:47 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  tools/env/Makefile | 11 ++++++++++-
>  tools/env/fw_env.h | 25 -------------------------
>  2 files changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/tools/env/Makefile b/tools/env/Makefile
> index ab73c8c..62a113a 100644
> --- a/tools/env/Makefile
> +++ b/tools/env/Makefile
> @@ -24,7 +24,7 @@
>  include $(TOPDIR)/config.mk
>
>  HOSTSRCS := $(SRCTREE)/lib/crc32.c  fw_env.c  fw_env_main.c
> -HEADERS        := fw_env.h $(OBJTREE)/include/config.h
> +HEADERS        := fw_env.h

I think this is the wrong approach.  We depend on the config.h being
included and the entire default env being available.  If you want to
get this behavior, I suggest you detect if there is a configured
board, and if so, include the config.h, and if not, bake in the bit
you need to cope with not having one.

>  # Compile for a hosted environment on the target
>  HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
> @@ -33,6 +33,15 @@ HOSTCPPFLAGS  = -idirafter $(SRCTREE)/include \
>                 -DUSE_HOSTCC \
>                 -DTEXT_BASE=$(TEXT_BASE)
>
> +# Pass CONFIG_xxx settings via the command line so that we can build w/out
> +# a config.h file existing in the first place.  Useful for generic builds.
> +CONFIG_VARS_TO_PASS = \
> +       ENV_OVERWRITE \
> +       OVERWRITE_ETHADDR_ONCE \
> +       ETHADDR

This doesn't look very maintainable, and it doesn't even include the
variables currently used.

> +HOSTCPPFLAGS += \
> +       $(foreach x,$(CONFIG_VARS_TO_PASS),$(if $(CONFIG_$(x)),-DCONFIG_$(x)=$(CONFIG_$(x))))
> +
>  ifeq ($(MTD_VERSION),old)
>  HOSTCPPFLAGS += -DMTD_OLD
>  endif
> diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
> index a1a6807..19703c7 100644
> --- a/tools/env/fw_env.h
> +++ b/tools/env/fw_env.h
> @@ -21,15 +21,6 @@
>   * MA 02111-1307 USA
>   */
>
> -/* Pull in the current config to define the default environment */
> -#ifndef __ASSEMBLY__
> -#define __ASSEMBLY__ /* get only #defines from config.h */
> -#include <config.h>
> -#undef __ASSEMBLY__
> -#else
> -#include <config.h>
> -#endif
> -
>  /*
>   * To build the utility with the static configuration
>   * comment out the next line.
> @@ -52,22 +43,6 @@
>  #define DEVICE2_ENVSECTORS     2
>  #endif
>
> -#ifndef CONFIG_BAUDRATE
> -#define CONFIG_BAUDRATE                115200
> -#endif
> -
> -#ifndef CONFIG_BOOTDELAY
> -#define CONFIG_BOOTDELAY       5       /* autoboot after 5 seconds     */
> -#endif
> -
> -#ifndef CONFIG_BOOTCOMMAND
> -#define CONFIG_BOOTCOMMAND                                                     \
> -       "bootp; "                                                               \
> -       "setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "        \
> -       "ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "   \
> -       "bootm"
> -#endif
> -

I agree that some of this should be cleaned up, but not as a result of
removing the config.h.

>  extern int   fw_printenv(int argc, char *argv[]);
>  extern char *fw_getenv  (char *name);
>  extern int fw_setenv  (int argc, char *argv[]);
> --
> 1.7.12.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

NAK

Thanks,
-Joe

[U-Boot] [PATCH 3/3] fw_env: fix building w/out a config.h

[Stefan Roese]

On 12/15/2012 07:04 PM, Joe Hershberger wrote:

<big snip>

>> -#ifndef CONFIG_BOOTCOMMAND
>> -#define CONFIG_BOOTCOMMAND                                                     \
>> -       "bootp; "                                                               \
>> -       "setenv bootargs root=/dev/nfs nfsroot=${serverip}:${rootpath} "        \
>> -       "ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}:${hostname}::off; "   \
>> -       "bootm"
>> -#endif
>> -
> 
> I agree that some of this should be cleaned up, but not as a result of
> removing the config.h.
> 
>>  extern int   fw_printenv(int argc, char *argv[]);
>>  extern char *fw_getenv  (char *name);
>>  extern int fw_setenv  (int argc, char *argv[]);
>> --
>> 1.7.12.4
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
> 
> NAK

Mike, any chance that you might find some time to rework this patch for
mainline acceptance?

BTW: I'm using this patch for Yocto builds of the Linux environment
tools. Without it, building fails.

Thanks,
Stefan

[U-Boot] [PATCH 1/3] fw_env: fix type of len

[Joe Hershberger]

Hi Mike,

On Sat, Nov 10, 2012 at 11:47 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> This variable is assigned by a size_t, and is printed that way, but is
> incorrectly declared as an int.  Which means we get warnings:
> fw_env.c: In function 'fw_setenv':
> fw_env.c:409:5: warning: format '%zu' expects argument of type 'size_t',
>         but argument 3 has type 'int' [-Wformat]
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [U-Boot,1/3] fw_env: fix type of len

[Tom Rini]

On Sat, Nov 10, 2012 at 07:47:45PM -0000, Mike Frysinger wrote:

> This variable is assigned by a size_t, and is printed that way, but is
> incorrectly declared as an int.  Which means we get warnings:
> fw_env.c: In function 'fw_setenv':
> fw_env.c:409:5: warning: format '%zu' expects argument of type 'size_t',
> 	but argument 3 has type 'int' [-Wformat]
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

For the series, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121219/3843ecee/attachment.pgp>

[U-Boot] [U-Boot,1/3] fw_env: fix type of len

[Joe Hershberger]

Hi Tom,

On Wed, Dec 19, 2012 at 5:00 PM, Tom Rini <trini@ti.com> wrote:
> On Sat, Nov 10, 2012 at 07:47:45PM -0000, Mike Frysinger wrote:
>
>> This variable is assigned by a size_t, and is printed that way, but is
>> incorrectly declared as an int.  Which means we get warnings:
>> fw_env.c: In function 'fw_setenv':
>> fw_env.c:409:5: warning: format '%zu' expects argument of type 'size_t',
>>       but argument 3 has type 'int' [-Wformat]
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>
> For the series, applied to u-boot/master, thanks!

I NACKed the third in this series.  Did you not see it?

-Joe

[U-Boot] [U-Boot,1/3] fw_env: fix type of len

[Tom Rini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/20/12 00:47, Joe Hershberger wrote:
> Hi Tom,
> 
> On Wed, Dec 19, 2012 at 5:00 PM, Tom Rini <trini@ti.com> wrote:
>> On Sat, Nov 10, 2012 at 07:47:45PM -0000, Mike Frysinger wrote:
>> 
>>> This variable is assigned by a size_t, and is printed that way,
>>> but is incorrectly declared as an int.  Which means we get
>>> warnings: fw_env.c: In function 'fw_setenv': fw_env.c:409:5:
>>> warning: format '%zu' expects argument of type 'size_t', but
>>> argument 3 has type 'int' [-Wformat]
>>> 
>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org> Acked-by: Joe
>>> Hershberger <joe.hershberger@ni.com>
>> 
>> For the series, applied to u-boot/master, thanks!
> 
> I NACKed the third in this series.  Did you not see it?

Yeah, I missed that, sorry.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQ0x/nAAoJENk4IS6UOR1WJ+IP/2g/Gb25k5WwKLG9LJWGp+ve
h173Xt1GhMWB8tutmEd1zc4Gat/ZhFv8PO6SxlR/3RxTZzNkg7v74SBkNnglHQVa
cvYIgzb72QvpIl7gHITHA38XedcV0UjEIJmlV6alqbZ/z4XBlNr/5Z71lwmI6VdM
H+yopRxmxXAtthTHNfyTyF6+MVItlCY15RkKFTvv93Pof1CRtRLI1jL5+497VtbS
hi7Fao2N/bHBUVkjQDU3hKOyApTNJhMBgxPu9EZszeAYqNLCNMH6FT4HvUNkYIjT
OEyG8bfm5D+v4x79fmLuLwPRg+yaBKu4nVJe7pzeTsQKfM3loIvyoaci2DLm9afS
ybmf97tRafMBYYiOTGJoRH3LXubCs1NZNgQIwCcDC9q59qy1E+uxp2jhBx/xfqGF
ixlz3A+J0c9Ahfjybejf9Y1M3I/3Sbyx6/fJR7jZsQo/lJROkp4W+yJHcmSzLqQO
jufYVZNi5lYo73R11TUa2oXngoBNW7jY4A9JHSA06JoB+GHp8Ze6yTFZD0QviTkY
oAWNHMvTFqzvC4UAHLXOGiiBTuxVaCd8nOR2/z/BzcKEHJuTTJxwe0ABB/FU9o3b
wF1MxwONT3/kfYA5eDItbjGaSa6NyY/66czjFSZjO0YJFZEKwKtLigwcgKw6Krq+
Mfv8c0WC0IXzgYikHFMm
=Fiob
-----END PGP SIGNATURE-----