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

Improve Debug impls #189

Merged
merged 2 commits into from
Sep 9, 2021
Merged

Improve Debug impls #189

merged 2 commits into from
Sep 9, 2021

Conversation

mkroening
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #189 (1d0388b) into master (4115924) will increase coverage by 0.10%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #189      +/-   ##
==========================================
+ Coverage   30.76%   30.86%   +0.10%     
==========================================
  Files          16       16              
  Lines        4018     4004      -14     
==========================================
  Hits         1236     1236              
+ Misses       2782     2768      -14     
Impacted Files Coverage Δ
src/linux/uhyve.rs 51.09% <0.00%> (-2.69%) ⬇️
src/linux/virtio.rs 9.04% <0.00%> (-0.10%) ⬇️
src/vm.rs 29.98% <0.00%> (+1.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4115924...1d0388b. Read the comment docs.

@stlankes
Copy link
Collaborator

stlankes commented Sep 7, 2021

Personal, I like my fmt::Debug implementation. I like to read hexadecimal numbers...

@mkroening
Copy link
Member Author

Fair enough! The derived impl supports that as well for the whole struct using "{:x?}". Is that sufficient, or would you like a different behavior per field?

@jounathaen
Copy link
Member

@mkroening's approach is pretty elegant though. Can you maybe post a quick example output of {:x} for comparison?

@mkroening
Copy link
Member Author

Sure!

Current output of println!("{:?}", BootInfo::new()) (including the trailing newline, further formatting flags are ignored):

magic_number 0xc0decafe
version 0x1
base 0x0
limit 0x0
tls_start 0x0
tls_filesz 0x0
tls_memsz 0x0
image_size 0x0
current_stack_address 0x0
current_percore_address 0x0
host_logical_addr 0x0
boot_gtod 0x0
mb_info 0x0
cmdline 0x0
cmdsize 0x0
cpu_freq 0
boot_processor 4294967295
cpu_online 0
possible_cpus 0
current_boot_id 0
uartport 0x0
single_kernel 1
uhyve 0

New output of println!("{:?}", BootInfo::new()):

BootInfo { magic_number: 3235826430, version: 1, base: 0, limit: 0, image_size: 0, tls_start: 0, tls_filesz: 0, tls_memsz: 0, current_stack_address: 0, current_percore_address: 0, host_logical_addr: 0, boot_gtod: 0, mb_info: 0, cmdline: 0, cmdsize: 0, cpu_freq: 0, boot_processor: 4294967295, cpu_online: 0, possible_cpus: 0, current_boot_id: 0, uartport: 0, single_kernel: 1, uhyve: 0, hcip: [255, 255, 255, 255], hcgateway: [255, 255, 255, 255], hcmask: [255, 255, 255, 0] }

New output of println!("{:#x?}", BootInfo::new()):

BootInfo {
    magic_number: 0xc0decafe,
    version: 0x1,
    base: 0x0,
    limit: 0x0,
    image_size: 0x0,
    tls_start: 0x0,
    tls_filesz: 0x0,
    tls_memsz: 0x0,
    current_stack_address: 0x0,
    current_percore_address: 0x0,
    host_logical_addr: 0x0,
    boot_gtod: 0x0,
    mb_info: 0x0,
    cmdline: 0x0,
    cmdsize: 0x0,
    cpu_freq: 0x0,
    boot_processor: 0xffffffff,
    cpu_online: 0x0,
    possible_cpus: 0x0,
    current_boot_id: 0x0,
    uartport: 0x0,
    single_kernel: 0x1,
    uhyve: 0x0,
    hcip: [
        0xff,
        0xff,
        0xff,
        0xff,
    ],
    hcgateway: [
        0xff,
        0xff,
        0xff,
        0xff,
    ],
    hcmask: [
        0xff,
        0xff,
        0xff,
        0x0,
    ],
}

@stlankes
Copy link
Collaborator

stlankes commented Sep 9, 2021

This is Ok for me.

@mkroening
Copy link
Member Author

bors r=stlankes

@bors bors bot merged commit 9017584 into hermit-os:master Sep 9, 2021
@mkroening mkroening deleted the debug branch September 9, 2021 08:30
bors bot added a commit that referenced this pull request Sep 24, 2021
192: mem_size tests: Assert exact error r=jounathaen a=mkroening

Depends on #189 but is ready for review.

This ensures, that the intended error variant is returned.

Co-authored-by: Martin Kröning <mkroening@posteo.net>
bors bot added a commit that referenced this pull request Sep 24, 2021
192: mem_size tests: Assert exact error r=jounathaen a=mkroening

Depends on #189 but is ready for review.

This ensures, that the intended error variant is returned.

Co-authored-by: Martin Kröning <mkroening@posteo.net>
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

Successfully merging this pull request may close these issues.

None yet

3 participants