Git Tips

When you send a patch-set to the Linux Kernel’s Upstream, the reviews might include some suggestions that need to be altered in few specific patches. This requires moving your git commit history to a specific patch and correcting it. This, if not done correctly might lead to an increased amount of work. Today, in this blog post, I am going to share some Git tips which I frequently use to tackle the same issue.

The foremost thing is to work in branches. You should always dedicate a branch for a specific type of work. This helps you to reduce the overhead of moving back a lot to fix a particular patch. A new branch can be created using the following command:
git checkout -b <branch-name>

You can list all the branches present in your working directory using:
git branch -a

Once you feel that a branch’s work is done and it is rendered useless, you can easily delete it using:
git branch -d <branch-name>

You can also list the commit log of your current branch with the help of following command
git log --pretty=oneline --abbrev-commit

Now, moving forward to fixing a specific patch which involves re-basing. Git offers an interactive Re-base mechanism to fix specific patches. This can be done using the following commands:

Consider a case, where you want to fix a particular commit (say bbc569ef). You can move back to this particular commit using:
git rebase --interactive bbc569ef^

In the default editor, modify ‘pick’ to ‘edit’ in the line whose commit you want to modify. Make your changes and then stage them with
git add <file-name>

Now you can use
git commit --amend

to modify the commit and
git rebase --continue

to return back to the previous head commit.

These few tips help me a lot while working with the patch traffic. Hope this helps you all as well.

Rashika

Advertisements

Making the Kernel free of “missing-prototypes” – 2

Hello all. In this blog post, I will discuss about the hurdles that I faced while eliminating “missing-prototypes” warnings from the drivers/ section of the Kernel.

One major issue I faced frequently was choosing the correct header file. As mentioned in the previous post, while removing the warning for a particular symbol, if there are more than one files which use it then we move the prototype declaration to an appropriate header file.

There can be various ways of choosing a header file:-

– Pick a header which all the files(using the symbol) include and move the prototype declaration to that header file. (But there might be large number of files which are common, how to choose among them?)

– Pick a file which has the structure definitions of the parameters used by the function to remove the overhead of managing nested includes. (It might sometimes lead you to choose a more general header when you should use a more specific one?)

– Pick a header in which the function logically fits, such as one that already includes similar functions (especially other functions from the same source file). (But, the functions in the source file may have the function prototypes in more than one file.)

Not so simple, eh? 😛 However, according to me the last method for choosing the header is the most appropriate one because it helps to choose a more specific header rather than a general one.

Another issue points out at proper handling of nested includes. Sometimes, when you move a prototype declaration to an *appropriate* header, you might need to resolve header dependencies because the function arguments may use a structure defined in other header.

There are two solutions to this problem: either you include both the headers in all the files which contains that symbol; or you can include the required header in the header file you chose for moving the prototype declaration into. The latter makes the header file self-contained (i.e. if something in a header needs another header, the first header should include the second directly, so that a source file including the header doesn’t have to include additional headers.). It also makes it easier for maintainers to include the header files in the future. (else you need to spend time to understand the header dependencies and include all necessary files).

This week, I also got stuck in one particularly interesting puzzle which points out to the necessity to remove the “missing-prototypes” warning. I was trying to eliminate these warnings for drivers/gpu/ and noticed that radeon_reg.h and radeon_drv.h conflict due to incompatible definitions therefore making it difficult to add both the headers in a single file. If it was included, then it throws the following error:

drivers/gpu/drm/radeon/radeon_drv.h:849:0: warning: “RADEON_RB3D_DSTCACHE_CTLSTAT” redefined [enabled by default]
drivers/gpu/drm/radeon/radeon_reg.h:1627:0: note: this is the location of the previous definition
drivers/gpu/drm/radeon/radeon_drv.h:1057:0: warning: “R300_CP_RESYNC_ADDR” redefined [enabled by default]
drivers/gpu/drm/radeon/radeon_reg.h:3342:0: note: this is the location of the previous definition
drivers/gpu/drm/radeon/radeon_drv.h:1058:0: warning: “R300_CP_RESYNC_DATA” redefined [enabled by default]
drivers/gpu/drm/radeon/radeon_reg.h:3343:0: note: this is the location of the previous definition

I was therefore not able to add prototypes to radeon.h (which includes radeon_reg.h) or radeon_drv.h if a file that needs to include one already includes the other.

For eg. If I add the prototype declaration of function r600_cs_legacy_get_tiling_conf to either header file radeon.h or radeon_drv.h, then I have to add the corresponding header file to r600_cp.c or r600_cs.c. Hence, either r600_cp.c or r600_cs.c would end up including both.

This can be resolved by either removing the redundant and conflicting definitions from either radeon_reg.h or radeon_drv.h or making the definitions match so that the compiler does not complain.While this problem stands unresolved for now (waiting for maintainers’ response), but it does help us to notice certain minor nits that remain unnoticed in the kernel.

Woof!! It was interesting to tackle these challenges 🙂 But, there are many more to go.
Will keep you posted about more…Bye 🙂

Rashika