Potential change/improvement current ports?

[RKennedy9064]

I've been following you blog for a while now trying to understand more about Rust and OS development and it's been a huge help so far. I did all posts you have so far, so I started working on VGA and trying to figure out how it all works. One thing I've noticed is that a lot of the VGA registers have weird rules associated with them to remain backwards compatible. This leads to ports that are readonly, write only, read from one port and write to another port. With the current implementation of the ports, I found it weird have have a port that's read-only have a write command since it implements the PortReadWrite trait together.

Have you ever thought of breaking that trait out and allowing the current Port to read and write so it doesn't break functionality, but including a read-only and write only version of ports. I took a look at the current code and since I'm fairly new at Rust, there's probably a better way to do what I did, but if you have a chance, could you check out the changes I made and let me know what you think? I mainly like the fact that it removes the chance of accidentally writing to a port that's supposed to be read-only by specifying which type of port you'd like to use, with the Port struct remaining the same, as to not break anything with previous version.

Here's a simple gist I made showing how I adapted your code to get it working in your project. If you're interested let me know, I'd love to talk more about it.

https://gist.github.com/RKennedy9064/3446beb0057b4f5f271b3606f5c6ddc6

Thanks,

Ryan Kennedy

[phil-opp]

This seems like an useful addition, thanks for proposing it! Your code looks good, would you mind creating a pull request for this?

(I would probably name the new types PortReadOnly and PortWriteOnly so that they're listed next to the Port type in the alphabetical docs.)

[RKennedy9064]

Okay sounds good. Glad you like it. I should have time tonight to make a pr and I'll make sure to use your naming for the port types.

[phil-opp]

Great, thank you!

[RKennedy9064]

@phil-opp Do I need rights in order to create a pull request? I made a branch off of master for the changes, but I get a 403 error when I try to push it. I also only see master using the New pull request button on Github. Let me know if I need to do anything special to make a pull request.

[phil-opp]

You need to create your own copy of the repository by forking it (using the fork button at the top). Then you can add the fork as a remote to your local repository:

git remote add fork url-of-your-fork

Then you can push your branch to your fork:

git push -u fork HEAD

Then you can open a pull request from your branch in RKennedy9064/x86_64 to rust-osdev/x86_64.

See https://guides.github.com/activities/forking/ for more information.

[RKennedy9064]

Thanks for the information. I Just add a pull request for the changes I made.

[phil-opp]

To link the pull request in question: #66