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

clippy #88

Merged
merged 29 commits into from
Jan 23, 2019
Merged

clippy #88

merged 29 commits into from
Jan 23, 2019

Conversation

Orycterope
Copy link
Member

@Orycterope Orycterope commented Dec 14, 2018

Fixes #68

clippy lint

@roblabla
Copy link
Member

roblabla commented Dec 14, 2018

MFW there's an xclippy command.

So, clippy has something that turns all warnings into errors. See https://github.com/MegatonHammer/linkle/blob/master/ci/script.sh#L16 (-D warnings means "deny warnings"). When we add it to the ci, we should use that.

@roblabla
Copy link
Member

Closes #68

@todo
Copy link

todo bot commented Dec 15, 2018

map_grub_module panic

map_grub_module panics if virtual space is exhausted, but should return an error instead, as it is used in the panic handler itself.


https://github.com/roblabla42/KFS/blob/57bcde27aab4019310edf15cf2ddab6bd9dcf10d/kernel/src/elf_loader.rs#L40-L45


This comment was generated by todo based on a TODO comment in 57bcde2 in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Dec 15, 2018

VirtalSpaceLand start_table/ end_table is arch specific

These functions should be moved to `paging::arch::i386::table.rs`


https://github.com/roblabla42/KFS/blob/57bcde27aab4019310edf15cf2ddab6bd9dcf10d/kernel/src/paging/lands.rs#L22-L26


This comment was generated by todo based on a TODO comment in 57bcde2 in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Dec 17, 2018

Land::contains_region() should not panic on 0 length

This function should return an error, as it really is likely someone (I) will call it at some point not expecting it can panic.


https://github.com/roblabla42/KFS/blob/d429d6ac276b6c1d1c1c82ad89cff006fedb34c3/kernel/src/paging/lands.rs#L49-L54


This comment was generated by todo based on a TODO comment in d429d6a in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Dec 17, 2018

move KernelLand Userland RTL to arch-specific paging

They are arch dependant, we should stop trying defining them in an agnostic way, even if they are expected to be the same for 32 bits architectures.
Especially for the Recursive Tables Land. Even if it's a agnostic concept, its size in virtual memory is mmu-dependant, and defined by the number of levels the mmu uses.


https://github.com/roblabla42/KFS/blob/d429d6ac276b6c1d1c1c82ad89cff006fedb34c3/kernel/src/paging/lands.rs#L84-L94


This comment was generated by todo based on a TODO comment in d429d6a in #88. cc @Orycterope.

Orycterope and others added 12 commits December 22, 2018 23:52
We used to break alignment requirements in elf_loader::get_kacs. To
avoid the issue, get_kacs now simply returns a byte array, and
ProcessCapabilities::parse uses byteorder to extract the u32 kacs
from it.
- Rewrite EntryOptions using bit_flags. Add some type safety around
  the Gate Type and Privilege Level.
- Manually implement Debug on IdtEntry, treating it like an enum
  based on the present flag and gate type, and joining the pointer
  fields.
- Manually implement Debug on Idt to deal with lack of Debug on large
  arrays.
More type safety, and better debug implementation. While we're at it,
make SegmentSelector aware of the TI flag (called is_ldt here).
While a DT entry's limit is the byte length - 1, the DTP's limit is the full
byte length. When converting back to a Vec<DTE>, I would probably end up losing
the entry.
Missing modules are: ipc, devices and frame_allocator.
This allows clippy to proceed on the full project.
@todo
Copy link

todo bot commented Jan 16, 2019

Feed the timer handler into a kernel preemption handler.

https://github.com/roblabla42/KFS/blob/490bc423cdec675607e7440010e4fdfba7546769/kernel/src/interrupts/irq.rs#L15-L25


This comment was generated by todo based on a TODO comment in 490bc42 in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Jan 16, 2019

Missing argument slot for SVCs on i386 backend

Our i386 SVC ABI is currently fairly different from the ABI used by Horizon/NX. This is for two reasons:

  1. We are missing one argument slot compared to the official SVCs, so we changed the ABI to work around it.
  2. The Horizon ABI "skipping" over some register is an optimization for ARM, but doesn't help on i386.
    That being said, there is a way for us to recover the missing SVC slot. We are currently "wasting" x0 for the syscall number. We could avoid this by instead using different IDT entries for the different syscalls. This is actually more in line with what the Horizon/NX kernel is doing anyways.
    Once we've regained this missing slot, we'll be able to make our ABI match the Horizon/NX 32-bit ABI. While the "skipping over" doesn't help our performances, it doesn't really hurt it either, and having a uniform ABI across platforms would make for lower maintenance.

https://github.com/roblabla42/KFS/blob/490bc423cdec675607e7440010e4fdfba7546769/kernel/src/interrupts/syscalls.rs#L549-L559


This comment was generated by todo based on a TODO comment in 490bc42 in #88. cc @Orycterope.

Orycterope and others added 2 commits January 17, 2019 19:39
Added documenting for
* pic
* pit
* rs232
* frame_allocator
* main
* ...
@roblabla
Copy link
Member

Missing:

  • libuser (nearly half of the errors are from libuser now)
  • kernel::ipc
  • userspace apps

We're almost there \o/

Only thing missing: vi has some cast warnings I'm not sure how to fix.
@todo
Copy link

todo bot commented Jan 20, 2019

Meme for KFS5.

We cannot give KFS5 until we have a meme. It is of utmost importance that a meme is found and placed here.


https://github.com/roblabla42/KFS/blob/8394882bf04c794370a55505f6c824173f82cae9/shell/src/main.rs#L205-L210


This comment was generated by todo based on a TODO comment in 8394882 in #88. cc @Orycterope.

@roblabla
Copy link
Member

We're almost there. Only a few warnings in vi are left. I'm going to fix them tomorrow when my head is fresh because they might be tricky and be pointing to actual bugs...

@todo
Copy link

todo bot commented Jan 22, 2019

Write documentation for AHCI

Documentation lints are disabled on AHCI for now, to avoid conflicts. Get @Orycterope to write some doc \o/


https://github.com/roblabla42/KFS/blob/fbd5755c39fed372ed43ba3e0db19685396ee041/ahci/src/main.rs#L13-L20


This comment was generated by todo based on a TODO comment in fbd5755 in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Jan 22, 2019

Review Terminal::new cast_sign_loss and cast_possible_wrap

Ping @Orycterope. I'm pretty sure they're correct but just want to make sure it looks good with you since it's originally your code.


https://github.com/roblabla42/KFS/blob/fbd5755c39fed372ed43ba3e0db19685396ee041/libuser/src/terminal.rs#L70-L77


This comment was generated by todo based on a TODO comment in fbd5755 in #88. cc @Orycterope.

@todo
Copy link

todo bot commented Jan 22, 2019

Review Terminal::display_glyph_in_box cast_sign_loss and cast_possible_wrap

Ping @Orycterope. I'm pretty sure they're correct but just want to make sure it looks good with you since it's originally your code.


https://github.com/roblabla42/KFS/blob/fbd5755c39fed372ed43ba3e0db19685396ee041/libuser/src/terminal.rs#L248-L258


This comment was generated by todo based on a TODO comment in fbd5755 in #88. cc @Orycterope.

Copy link
Member

@marysaka marysaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

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.

3 participants