Hi Stefano On Thu, Jun 7, 2018 at 9:15 AM Stefano Babic <sba...@denx.de> wrote: > > Hi Alex, > > On 07/06/2018 10:07, Alex Kiernan wrote: > > Hi Stefano > > > > On Thu, Jun 7, 2018 at 8:16 AM Stefano Babic <sba...@denx.de> wrote: > >> > >> Hi Alex, > >> > >> On 06/06/2018 18:33, Alex Kiernan wrote: > >>> The line length limit that fw_{printenv,setenv} impose (1024 > >>> characters) has tripped me up twice in as many days, so I figured I'd > >>> rewrite it to use getline as we already have that in tools/. > >>> > >>> But in looking how I structure that change, I've immediately run into > >>> questions... > >>> > >>> Right now I have a horrid hack that just pulls in ../getline.[ch], but > >>> I'm thinking I should hoist the build of the environment tools, into > >>> tools/Makefile from tools/env/Makefile, move getline.c into tools/lib > >>> and then patch the trivial change in to swap from fgets to getline in > >>> then. > >> > >> I do not understand the point: fgets has not such as limit, it simply > >> pus bytes into a preallocated buffer. And for scripts (the only point > >> where fgets is called), yes, the buffer is 1024. This is the only place > >> where fgets() is called. > >> > > > > There's two - one is this (where I have a line longer than 1024) and a > > second when it's parsing the config file (128 chars), where I overran > > it because of a mount point which ends up named according to a > > SHA-256. > > ok > > > In the second case the long line wasn't detected and in my > > case I spent some time before I figured out that the reason > > fw_printenv was claiming CRC fail was because it hadn't read the > > entire line and thought the environment buffer was shorter than it > > actually was. > > Understood, thanks. > > > > >> On the other side, getline allocates the buffer. In our case, we have > >> couple of <var> <value> , and it looks strange we cannot have this under > >> control. If you need this, you have very long value for a variable, and > >> this makes the script hard to read. It is good practise to split the > >> long assignment in more variables to improve maintainance. > >> > > > > I'm dealing with migrating from an ancient version of U-Boot, to a new > > version and got tripped up trying to manipulate the existing > > environment, so it's not under my control at this point. So whilst I'd > > like to not have long variables, I'd also like to not change it at the > > point of migration, though probably I can just remove the existing > > lines before writing things back as I'm not actually touching those > > ones. > > > > Right. > > >>> > >>> I'm aware this also gets used as a library, so I'm guessing I need to > >>> make linking of getline.c conditional somehow. > >> > >> But getline is part of glibc. Why do you have to copy it ? > >> > > > > I wasn't clear that assuming glibc (or musl which also has it) was an > > acceptable option. If we can assume getline exists, then the patch is > > trivial - that it exists in tools/getline.c made me think that wasn't > > a reasonable assumption. > > The tools fw_{printenv,getenv} run in user space and they are using a > standard environment, that is you can assume that glibc | musl is > linked. In fact, you see other function calls that are part of libc. > > To let you better understand, just fw_env_main.c is not part of the > library. >
Thanks, that helps clarify. -- Alex Kiernan _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot