Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Routine of RAM size evaluation not sufficient and might cause issues #106

Closed
luddinho opened this issue Jun 30, 2023 · 2 comments
Closed

Comments

@luddinho
Copy link

Environment

script version: v3.0.56
OS: DSM 7.2
Device: 1817+
RAM: 2x 8GB

Problem description

The evaluation of RAM size might cause issues because the condition of the grep command for dmidecode is not clear enough.

Example:
On my system the command dmidecode -t memory | grep "[Ss]ize" will return the following content.

Size: 8192 MB
Size: No Module Installed
Size: 8192 MB
Size: No Module Installed

Now you can imagine what will happen with the variables $ramsize and $bytes when evaluating with awk.

ramsize=$(printf %s "${array[num]}" | awk '{print $2}')
bytes=$(printf %s "${array[num]}" | awk '{print $3}')

The result is:

ramsize: 8192
bytes: MB
ramsize: No
bytes: Module
ramsize: 8192
bytes: MB
ramsize: No
bytes: Module

At the end of the loop we will have fortunately the correct numeric value for the variable $ramtotal. But the unit for $bytes is inconsitent.

ramtotal: 16384
bytes: Module

Proposed solution

My proposal is to make the grep command with an extended regular expression much more precise that will return only the size with numeric values and additionally with a valid unit either MB or GB as postfix at the end of the line.

#Proposal
dmidecode -t memory | grep -E "[Ss]ize: [0-9]+ [MG]{1}[B]{1}$"

# Return
Size: 8192 MB
Size: 8192 MB

I haven´t checked other systems yet, but it could also lead to an error in the calculation of $ramtotal size.
Please have a look and check if the improvement can be implemented.

@007revad
Copy link
Owner

Nicely formatted issue, and nice solution. Thank you.

007revad added a commit that referenced this issue Jun 30, 2023
v3.1.57
- Added enabling E10M20-T1, M2D20 and M2D18 for DS1821+, DS1621+ and DS1520+.
- Fixed enabling E10M20-T1, M2D20 and M2D18 cards in models that don't officially support them.
- Fixed bugs where the calculated amount of installed memory could be incorrect:
   - If last memory socket was empty an invalid unit of bytes could be used. Issue #106
   - When dmidecode returned MB for one ram module and GB for another ram module. Issue #107
@luddinho
Copy link
Author

From a code review so far, it looks pretty good. I think this is fine now.

Thanks for the fix.

@007revad 007revad mentioned this issue Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants