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

Code polishing #8

Open
aleksei-burlakov opened this issue Jun 25, 2019 · 9 comments
Open

Code polishing #8

aleksei-burlakov opened this issue Jun 25, 2019 · 9 comments

Comments

@aleksei-burlakov
Copy link

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
Copy link
Author

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

@aleksei-burlakov
Copy link
Author

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

@crazyss
Copy link
Owner

crazyss commented Jun 25, 2019

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
Copy link
Owner

crazyss commented Jun 25, 2019

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
Copy link
Owner

crazyss commented Jun 25, 2019

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

crazyss commented Jun 26, 2019

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?

wjn740 pushed a commit to wjn740/hypervisor_last that referenced this issue Jun 26, 2019
We have an agreement at using kernel code style.
crazyss#8

So I porting a script at script/Lindent from kernel source tree.
and apply it on all *.[ch] files, Assembly file doesn't do.
wjn740 pushed a commit to wjn740/hypervisor_last that referenced this issue Jun 26, 2019
We have an agreement at using kernel code style.
crazyss#8

So I porting a script at script/Lindent from kernel source tree.
and apply it on all *.[ch] files, Assembly file doesn't do.
@aleksei-burlakov
Copy link
Author

I have created a PR #13, please have a look.

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

No branches or pull requests

2 participants