IzzySoft / Adebar

Android DEvice Backup And Report, using Bash and ADB. Moved to https://codeberg.org/izzy/Adebar -- this is only a mirror now.
GNU General Public License v2.0
246 stars 41 forks source link

partitions.lib line 221/1024 syntax error under Windows Bash #41

Closed Catfriend1 closed 6 years ago

Catfriend1 commented 6 years ago

Describe the bug The script fails to generate the documentation partly.

To Reproduce Steps to reproduce the behavior: Run $ ./adebar-cli _mi8

Precise error message (if any): Adebar running: Gathering lists of installed apps Obtaining list of disabled apps Creating script to enable ALL apps Creating backup/restore scripts for UserApps Creating backup/restore scripts for SytemApps Checking default install-location Collecting partition details Collecting device information

./lib/partitions.lib: line 221: / 1024: syntax error: operand expected (error token is "/ 1024") ./lib/partitions.lib: line 221: / 1024: syntax error: operand expected (error token is "/ 1024")

{repeated 80 times more}

./lib/partitions.lib: line 221: / 1024: syntax error: operand expected (error token is "/ 1024") ./lib/partitions.lib: line 221: / 1024: syntax error: operand expected (error token is "/ 1024") ./lib/partitions.lib: line 221: / 1024: syntax error: operand expected (error token is "/ 1024") ")syntax error: invalid arithmetic operator (error token is " ")syntax error: invalid arithmetic operator (error token is " Pulling configuration files Generating app detail info PostProcessing and Cleanup Adebar done.

Expected behavior The partitions documentation should be generated without error.

Desktop (please complete the following information):

Smartphone (please complete the following information):

IzzySoft commented 6 years ago

Two details I need to get this fixed:

  1. does the "Service State" Markdown really appear on the console? Oh, indeed it did (fixed with 621e4ed); why didn't you tell me before? :wink:
  2. From the device info document, what "source" is given for the (failing) partition info? Depending on that, can you check with lib/partitions.lib which shell command the details are based on, and provide me with its output (by running the command manually with adb shell)? From the number of times it is reported, it seems to be a valid source – but the output format has a different structure than expected.

My guess/hope is it's not in _fsProcByName() – as there I had the same issue with my BQ Aquaris X5 Plus and already fixed it. Must be one of the other sources needing the same cure – we just need to find out which.

Catfriend1 commented 6 years ago

@IzzySoft I updated to the latest commit in your repo and ran again.

Storage details

  • Storage encrypted: encrypted
  • Partitions (source: BYNAME)
  • /dev/block/sdf4 : (ImageFv) ( MiB)
  • /dev/block/sde32 : (abl_a) ( MiB)
  • /dev/block/sde33 : (abl_b) ( MiB)
  • /dev/block/sde14 : … (long list of more partitions here)
  • /dev/block/sdb1 : (xbl_config_a) ( MiB)
  • /dev/block/sdc1 : (xbl_config_b) ( MiB)
  • Disk statistics:
  • Latency: 0ms [512B Data Write]
  • Data-Free: 104807520K 116193656K total = 90% free
  • Cache-Free: 104807520K 116193656K total = 90% free
  • System-Free: 302172K 2982364K total = 10% free
  • File-based Encryption: true
Catfriend1 commented 6 years ago

Replacing the line of the error with

echo ${MTD_DEV[${counter}]} -- ${pname} -- ${MTD_SIZE[${counter}]}

I found out it's having those vars set: `: (dip) ( MiB)ck/sde28

-- dsp --/sde44

./lib/partitions.lib: line 228: / 1024: syntax error: operand expected (error token is "/ 1024") `

Catfriend1 commented 6 years ago

@IzzySoft https://pastebin.com/YbnkNawm

IzzySoft commented 6 years ago

Uh … it uses _fsProcByName() which I just fixed for my BQ device (and kept it working for previous ones, tested with mine). Unfortunately, your pastebin just gives the output from adb ${ADBOPTS} shell cat /proc/mounts. Coud you please check via adb shell and su whether there is

Besides: No need to post the entire list each time (just makes the issue hard to read). You can always abbreviate that (see the edits I'm now about to make to your comments above :wink:)

Catfriend1 commented 6 years ago

I don't have root but still ran the commands: https://pastebin.com/Snnne0aF

IzzySoft commented 6 years ago

OK, 2 immediate perceptions:

Thanks to your previous pastebin, I can now check what went wrong with the sizes. Strange thing, output from cat /proc/partitions looks similar to that of my devices, so the position of the size is the same.

Now, a quick hack would be not to show the size if it's not there – but I don't like that. We want the size there. So we need to debug why it's not found. Lacking your device here, I must ask you for that. Please modify lib/partitions.lib:

I've just verified with your output from the first pastebin:

$ foo=(   1        0       8192 ram0)
$ echo ${foo[2]}
8192

So the logic should fit. So my assumption is that some grep failed. Let's see what your output is (cut the partitions part from within the loop to a reasonable number, some examples should suffice).

Catfriend1 commented 6 years ago

@IzzySoft https://pastebin.com/zCcLn8Wg

Catfriend1 commented 6 years ago

Ok, fixed it myself by this commit: 363dcb3 "### Storage details" documentation part is now created correctly.

Output

Adebar running: Gathering lists of installed apps Obtaining list of disabled apps Creating script to enable ALL apps Creating backup/restore scripts for UserApps Creating backup/restore scripts for SytemApps Checking default install-location Collecting partition details Collecting device information Service state: Service state: ")syntax error: invalid arithmetic operator (error token is " ")syntax error: invalid arithmetic operator (error token is " Pulling configuration files Generating app detail info PostProcessing and Cleanup Adebar done.

IzzySoft commented 6 years ago

Funny – but well spotted, thanks!

As for your diff: I intentionally avoid calls to external helpers (here: sed) whenever possible. Check the trim() function in lib/common.lib for the matching Bashism:

var="${var//[$'\r']}"

should remove all CRs (there are none expected apart the ones at line's end). So instead of your sed line, could you try

MTD_DEV[${counter}]="${MTD_DEV[${counter}]//[$'\r']}"

The added quotes ("${tmp2}") are a good idea as well, of course :wink:

If that then works for you, I'd cross-check with my devices. Shouldn't break a thing – so if it doesn't break a thing, I'll happily accept your PR.

IzzySoft commented 6 years ago

PS: I've just tried that, works for me (but then, for me it worked before as well). Let me know when you've tested it and can confirm, so I push it.

Catfriend1 commented 6 years ago

PR updated.

IzzySoft commented 6 years ago

PR merged, thanks again!

Now I'd only wish to know what throws the ")syntax error: invalid arithmetic operator (error token is " on your end (the two "Service State" echos are already fixed).