Get only the first CONFIG_ENV_SECT_SIZE

[amdrsantos]

In the u-boot, some platforms configs have the "CONFIG_ENV_SECT_SIZE" referenced more than once in its .h.
This can causes the "build-config/scripts/onie-mk-bin.sh" to fail.

In my use case:
++ grep CONFIG_ENV_SECT_SIZE /home/build/src/onie/build/_-r0/u-boot/u-boot-2013.01.01/include/configs/.h
++ awk '{print $3}'

• env_sector_size='0x20000
  (CONFIG_SYS_MONITOR_BASE
  "\0"'
  /home/build/src/onie/build-config/scripts/onie-mk-bin.sh: line 80: 0x20000
  (CONFIG_SYS_MONITOR_BASE
  "\0" + 0 : syntax error in expression (error token is "(CONFIG_SYS_MONITOR_BASE
  "\0" + 0 ")
• '[' '0x20000
  (CONFIG_SYS_MONITOR_BASE
  "\0"' = '' ']'
• '[' '0x20000
  (CONFIG_SYS_MONITOR_BASE
  "\0"' = 0 ']'

Therefore I suggest adding the "-m 1" parameter to grep, in order to get only the first match.

Cheers,
Alex

[ehdoyle]

Hi - thanks for pointing this issue out, and the pull request makes it a lot clearer what the code would look like.
As I'm starting to dig into it, my initial thought is: should those platforms be defining it more than once? Seems like that would be a bug that those platforms should fix, unless I (quite possibly) am missing something there.
That said, I still think it'd be a good idea to catch and call out multiple definitions, rather than having the code fail with multiple matches.
Do you have an example or two of the platforms where this happens? (I'm looking at a slew of patch files for different platforms so I can only really be sure what the final state will be by building them one at a time)
Thanks!

[amdrsantos]

Hi,
I looked into the repository, and I could find:
https://github.com/opencomputeproject/onie/blob/fc20b7ee376f70750b95fa25941202f22c81a581/machine/qemu_armv7a/u-boot/platform-qemu_armv7a.patch
https://github.com/opencomputeproject/onie/blob/fc20b7ee376f70750b95fa25941202f22c81a581/machine/alphanetworks/alphanetworks_snq6070_320f/u-boot/platform-alpha-networks-snq6070_320f.patch
https://github.com/opencomputeproject/onie/blob/fc20b7ee376f70750b95fa25941202f22c81a581/machine/alphanetworks/alphanetworks_snx6070_486f/u-boot/platform-alpha-networks-snx6070_486f.patch
https://github.com/opencomputeproject/onie/blob/fc20b7ee376f70750b95fa25941202f22c81a581/machine/lenovo/lenovo_g8296/u-boot/platform-lenovo-g8296.patch

But there is https://github.com/opencomputeproject/onie/blob/c224c29b9abd48c58b5fb5b40bce4c8ab0d8e805/machine/nxp/u-boot/0045-board-ls1012a-FRWY-LS1012A-board-support.patch
which makes me think that the grep needs to be more strict, like:
grep -m 1 -E "(#define CONFIG_ENV_SECT_SIZE)"

Regards

[ehdoyle]

Hmm. For a minor change, there's a surprising amount of corner cases.
What was the system(s) you were building for, so that I can test against them.
I'm thinking this should cover it, or fail if there's multiple definitions: Thoughts?

`if [ -z "$env_sector_size" ] ; then
# try to figure it out from the $MACHINE_CONIFG

# find all #defines                                                                    
env_sector_define=$(grep -E \#define[[:space:]]*CONFIG_ENV_SECT_SIZE $MACHINE_CONFIG ) 
# Not found at all, or found more than once                                            
if [ $(echo "$env_sector_define" | wc -l ) != 1 ];then                                 
    echo "ERROR: #define CONFIG_ENV_SECT_SIZE was found\                               

$(echo "$env_sector_define" | wc -l ) times in $MACHINE_CONFIG. $env_sector_define"
exit 1
else
env_sector_size=$( echo "$env_sector_define" | awk '{print $3}' )
echo "Got $env_sector_size"
fi
# env_sector_size=$(grep CONFIG_ENV_SECT_SIZE $MACHINE_CONFIG | awk '{print $3}')
fi
# falls through to existing code
env_sector_size=$(( $env_sector_size + 0 ))
if [ "$env_sector_size" = "" ] || [ "$env_sector_size" = "0" ] ; then
echo "ERROR: Unable to find #define CONFIG_ENV_SECT_SIZE in $MACHINE_CONFIG."
exit 1
fi `

[amdrsantos]

I would include one more test, for the case that grep returns no results:

`if [ -z "$env_sector_size" ] ; then
# try to figure it out from the $MACHINE_CONIFG

# find all #defines                                                                    
env_sector_define=$(grep -E \#define[[:space:]]*CONFIG_ENV_SECT_SIZE $MACHINE_CONFIG ) 
# Not found at all, or found more than once
if [ -z "$env_sector_define" ] ; then 
    echo "ERROR: #define CONFIG_ENV_SECT_SIZE was not found in $MACHINE_CONFIG."  
    exit 1
elif [ $(echo "$env_sector_define" | wc -l ) != 1 ]; then                                 
    echo "ERROR: #define CONFIG_ENV_SECT_SIZE was found\                               
    $(echo "$env_sector_define" | wc -l ) times in $MACHINE_CONFIG. $env_sector_define"
    exit 1
else
    env_sector_size=$( echo "$env_sector_define" | awk '{print $3}' )
    echo "Got $env_sector_size"
fi
env_sector_size=$(grep CONFIG_ENV_SECT_SIZE $MACHINE_CONFIG | awk '{print $3}')

fi `

I am working on a Linux debian9x64bit 4.9.0-8-amd64 #1 SMP Debian 4.9.130-2 (2018-10-27) x86_64 GNU/Linux.

The platform I am tring to integrated is similar to Accton AS6700-32X, can not disclosure yet.

[ehdoyle]

Nice catch on the empty value. I'd have thought the wc would have reported 0, but of course it's counting lines.
With this patch in I'll try rebuilding the machines listed where this may be an issue, and then do some other platform builds to make sure it doesn't break anything.