use 64 bits variable to store memory size #456

Merged
SlavekB merged 1 commits from freebsd-info-memory into master 2 months ago
denis commented 2 months ago
Collaborator

int is too small to store modern memory size

Signed-off-by: Denis Kozadaev denis@dilos.org

int is too small to store modern memory size Signed-off-by: Denis Kozadaev <denis@dilos.org>
denis added 1 commit 2 months ago
9ca6f9ac6c
use 64 bits variable to store memory size
Fat-Zer requested changes 2 months ago
Fat-Zer left a comment
Collaborator

Make sense, good catch, but there is one more thing...

Make sense, good catch, but there is one more thing...
char blah[10], buf[80], *used_str, *total_str;
/* Stuff for sysctl */
int memory;
unsigned long memory;
Collaborator

There is an snprintf() later which would require a format specifier change, but it and blah seems not to be used anyway, so it would be better to remove those both altogether...

There is an `snprintf()` later which would require a format specifier change, but it and `blah` seems not to be used anyway, so it would be better to remove those both altogether...
denis commented 2 months ago
Poster
Collaborator

Macro MEMORY() used to cast the variable to t_memory (aka unsigned long), there is no reason to change anything because the variable is already unsigned long type.

Macro MEMORY() used to cast the variable to t_memory (aka unsigned long), there is no reason to change anything because the variable is already unsigned long type.
denis commented 2 months ago
Poster
Collaborator

err: t_memsize instead of t_memory

err: t_memsize instead of t_memory
denis commented 2 months ago
Poster
Collaborator

by the way, the result of snprintf is not used anywhere and the line could be removed

by the way, the result of snprintf is not used anywhere and the line could be removed
Collaborator

by the way, the result of snprintf is not used anywhere and the line could be removed

yes, that's exactly what I'm talking about: remove snprintf and the buffer blah as they aren't used.

If it were used, it'd require to change the format %d->%ul

> by the way, the result of snprintf is not used anywhere and the line could be removed yes, that's exactly what I'm talking about: remove `snprintf` and the buffer `blah` as they aren't used. If it were used, it'd require to change the format `%d`->`%ul`
Fat-Zer marked this conversation as resolved
denis added 1 commit 2 months ago
54e9ec4d1f
remove unused snprintf
SlavekB reviewed 2 months ago
SlavekB left a comment
Owner

Please, you can add a prefix in the commit message, for example: kcontrol-info-memory: to make it clear to the git log, what is it?

Please, you can add a prefix in the commit message, for example: `kcontrol-info-memory:` to make it clear to the git log, what is it?
denis force-pushed freebsd-info-memory from 54e9ec4d1f to 8bc4276855 2 months ago
denis force-pushed freebsd-info-memory from 8bc4276855 to 220dec20f9 2 months ago
SlavekB requested review from Fat-Zer 2 months ago
Fat-Zer approved these changes 2 months ago
Fat-Zer left a comment
Collaborator

LGTM now (assuming Slavek's commit message notice will be fixed)

LGTM now (assuming Slavek's commit message notice will be fixed)
SlavekB approved these changes 2 months ago
SlavekB left a comment
Owner

It looks good, thank you.

It looks good, thank you.
Owner

Yes, committ message is now good. Although after the squash has disappeared information about the drop of unnecessary snprintf, but this is not essential.

Yes, committ message is now good. Although after the squash has disappeared information about the drop of unnecessary snprintf, but this is not essential.
SlavekB merged commit 220dec20f9 into master 2 months ago
SlavekB deleted branch freebsd-info-memory 2 months ago
SlavekB added this to the R14.1.2 release milestone 2 months ago

Reviewers

Fat-Zer approved these changes 2 months ago
SlavekB approved these changes 2 months ago
The pull request has been merged as 220dec20f9.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tdebase#456
Loading…
There is no content yet.