crazyss / hypervisor_last

upstream development the newest feature about HV.
10 stars 4 forks source link

Code polishing #8

Open aleksei-burlakov opened 5 years ago

aleksei-burlakov commented 5 years ago

I think the code needs to chose some well used coding convention (or make up our own) and format the code according to it. What is also worthy to do is to make the functions static that are static. There are many functions declared in the common.h file although they are not common, e.g. init_keyboard(), enable_mouse(), etc.

aleksei-burlakov commented 5 years ago

Besides, let's not use tabs, but only 4 spaces.

aleksei-burlakov commented 5 years ago

I would propose C++ code style from google https://google.github.io/styleguide/cppguide.html

crazyss commented 5 years ago

I would propose C++ code style from google https://google.github.io/styleguide/cppguide.html

I think we could choose kernel code style, we are not developing a CPP program.

crazyss commented 5 years ago

About code formatting, if we follow kernel's code style, kernel provides a "script" could help us to tidy all code. tidy up could be done at the end of this hackweek.

crazyss commented 5 years ago

About some function is 'static' or not. or something else. there is two groups issue:

  1. declare or define.
  2. static or no static.

Issue 1, yes we need clear them to declare or define. and rearrange them, I agree.

Issue 2, this is a tricky issue, it relative to "linker and loader" and "ld script". sometimes, also be impacted by memory layout and stack segment status. "static data" will put into an "initial data segment", no static might somewhere else. I did this for the sake of convenience. Over time, we began to understand these technical points step by step, and we can optimize them.

aleksei-burlakov commented 5 years ago

About code formatting, if we follow kernel's code style, kernel provides a "script" could help us to tidy all code. tidy up could be done at the end of this hackweek.

The kernel code style is perfect.

aleksei-burlakov commented 5 years ago

About some function is 'static' or not. or something else. there is two groups issue:

  1. declare or define.
  2. static or no static.

Issue 1, yes we need clear them to declare or define. and rearrange them, I agree.

Issue 2, this is a tricky issue, it relative to "linker and loader" and "ld script". sometimes, also be impacted by memory layout and stack segment status. "static data" will put into an "initial data segment", no static might somewhere else. I did this for the sake of convenience. Over time, we began to understand these technical points step by step, and we can optimize them.

I staticized all possible methods in the start.c file in my fork and moreover I made all possible variables local (i.e. allocated on stack instead of the heap) and all works as fine.

crazyss commented 5 years ago

About some function is 'static' or not. or something else. there is two groups issue:

  1. declare or define.
  2. static or no static.

Issue 1, yes we need clear them to declare or define. and rearrange them, I agree. Issue 2, this is a tricky issue, it relative to "linker and loader" and "ld script". sometimes, also be impacted by memory layout and stack segment status. "static data" will put into an "initial data segment", no static might somewhere else. I did this for the sake of convenience. Over time, we began to understand these technical points step by step, and we can optimize them.

I staticized all possible methods in the start.c file in my fork and moreover I made all possible variables local (i.e. allocated on stack instead of the heap) and all works as fine.

so cool, so would you mind share your code via a PR?

aleksei-burlakov commented 5 years ago

I have created a PR https://github.com/crazyss/hypervisor_last/pull/13, please have a look.