[U-Boot,01/11] fw_env.c: Increase max dev path to 32

Submitted by Tom Rini on Sept. 26, 2013, 8:27 p.m.

Details

Message ID 1380227287-26057-2-git-send-email-trini@ti.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Tom Rini Sept. 26, 2013, 8:27 p.m.
Currently our limit is too small to allow for /dev/mmcblk0boot0 to work,
for example.  Expand to 32 for future needs.

Signed-off-by: Tom Rini <trini@ti.com>
---
 tools/env/fw_env.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Wolfgang Denk Oct. 5, 2013, 8:02 p.m.
Dear Tom Rini,

In message <1380227287-26057-2-git-send-email-trini@ti.com> you wrote:
> Currently our limit is too small to allow for /dev/mmcblk0boot0 to work,
> for example.  Expand to 32 for future needs.
> 
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
>  tools/env/fw_env.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 577ce2d..bf5c552 100644
> --- a/tools/env/fw_env.c
> +++ b/tools/env/fw_env.c
> @@ -40,7 +40,7 @@
>  	_min1 < _min2 ? _min1 : _min2; })
>  
>  struct envdev_s {
> -	char devname[16];		/* Device name */
> +	char devname[32];		/* Device name */

Do we really need a static size here?  Can we not auto-adjust to the
needed size, say by dynamically allocating the buffer?

Best regards,

Wolfgang Denk
Tom Rini Oct. 6, 2013, 8:55 p.m.
On Sat, Oct 05, 2013 at 10:02:12PM +0200, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <1380227287-26057-2-git-send-email-trini@ti.com> you wrote:
> > Currently our limit is too small to allow for /dev/mmcblk0boot0 to work,
> > for example.  Expand to 32 for future needs.
> > 
> > Signed-off-by: Tom Rini <trini@ti.com>
> > ---
> >  tools/env/fw_env.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> > index 577ce2d..bf5c552 100644
> > --- a/tools/env/fw_env.c
> > +++ b/tools/env/fw_env.c
> > @@ -40,7 +40,7 @@
> >  	_min1 < _min2 ? _min1 : _min2; })
> >  
> >  struct envdev_s {
> > -	char devname[16];		/* Device name */
> > +	char devname[32];		/* Device name */
> 
> Do we really need a static size here?  Can we not auto-adjust to the
> needed size, say by dynamically allocating the buffer?

Doesn't look like it, without a big change to the parsing code.
Wolfgang Denk Oct. 7, 2013, 5:47 a.m.
Dear Tom,

In message <20131006205527.GP15917@bill-the-cat> you wrote:
> 
> > Do we really need a static size here?  Can we not auto-adjust to the
> > needed size, say by dynamically allocating the buffer?
> 
> Doesn't look like it, without a big change to the parsing code.

I don't think this requires a big change.  Eventually all it takes is
changing the sscanf() call in get_config() to use a format "%ms"
instead of plain "%s"; form the sscanf() man page:

        ยท An optional 'm' character. This is used with string
          conversions (%s, %c, %[), and relieves the caller of the
          need to allocate a corresponding buffer to hold the input:
          instead, scanf() allocates a buffer of sufficient size, and
          assigns the address of this buffer to the corresponding
          pointer argument, which should be a pointer to a char *
          variable (this variable does not need to be initialized
          before the call). The caller should subsequently free(3)
          this buffer when it is no longer required.

OK, the struct should then of course contain a const char pointer
instead of the buffer itself, but that's also a trivial change.

Best regards,

Wolfgang Denk
Tom Rini Oct. 7, 2013, 12:06 p.m.
On Mon, Oct 07, 2013 at 07:47:13AM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20131006205527.GP15917@bill-the-cat> you wrote:
> > 
> > > Do we really need a static size here?  Can we not auto-adjust to the
> > > needed size, say by dynamically allocating the buffer?
> > 
> > Doesn't look like it, without a big change to the parsing code.
> 
> I don't think this requires a big change.  Eventually all it takes is
> changing the sscanf() call in get_config() to use a format "%ms"
> instead of plain "%s"; form the sscanf() man page:
> 
>         ?? An optional 'm' character. This is used with string
>           conversions (%s, %c, %[), and relieves the caller of the
>           need to allocate a corresponding buffer to hold the input:
>           instead, scanf() allocates a buffer of sufficient size, and
>           assigns the address of this buffer to the corresponding
>           pointer argument, which should be a pointer to a char *
>           variable (this variable does not need to be initialized
>           before the call). The caller should subsequently free(3)
>           this buffer when it is no longer required.
> 
> OK, the struct should then of course contain a const char pointer
> instead of the buffer itself, but that's also a trivial change.

Well, that's what I get for looking at the code, and not checking man
pages.  Agreed, I'll re-work this part.

Patch hide | download patch | download mbox

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 577ce2d..bf5c552 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -40,7 +40,7 @@ 
 	_min1 < _min2 ? _min1 : _min2; })
 
 struct envdev_s {
-	char devname[16];		/* Device name */
+	char devname[32];		/* Device name */
 	ulong devoff;			/* Device offset */
 	ulong env_size;			/* environment size */
 	ulong erase_size;		/* device erase size */