Top.Mail.Ru
? ?

Previous Entry | Next Entry

on static function declaration

If you are a seasoned C programmer, skip this post entirely (or try to find bugs in it). If you know C but don't consider yourself an expert, please read on -- it might be helpful.

I was working a bit on vzctl today (my target was bug #1757, which is still a work in progress) and ... I am not sure how, but I ended up declaring most functions in src/vzlist.c as static. I thought it doesn't have any practical value -- I was wrong!

In C, if you declare the function as static, it means its visibility is limited to the translation unit (i.e. a file) in which it is defined. In other words, you can only call/use a static function from another function in the same file.

Now, in vzctl sources vzlist.c is only linked to one binary -- vzlist, and therefore I thought it doesn't make much sense to declare functions as static. Nevertheless I did it (see git commit).

Next thing I got is a set of compiler warnings! OK, all right, let's take a look...

First set of warnings is self-explanatory. See:
vzlist.c:825:14: warning: ‘parse_var’ defined but not used
vzlist.c:1075:14: warning: ‘remove_sp’ defined but not used
vzlist.c:1357:12: warning: ‘get_stop_quota_stats’ defined but not used


Easy! In some ancient time, these functions were used, now the code has changed and no one needs these three, but they were not removed for some reason (probably just forgotten). Solution: remove the dead code (see git commit).

Second set of warnings looks similar:
vzlist.c:400:1: warning: ‘dcachesize_m_sort_fn’ defined but not used
vzlist.c:400:1: warning: ‘dcachesize_l_sort_fn’ defined but not used
vzlist.c:400:1: warning: ‘dcachesize_b_sort_fn’ defined but not used
vzlist.c:400:1: warning: ‘dcachesize_f_sort_fn’ defined but not used
vzlist.c:411:1: warning: ‘diskinodes_s_sort_fn’ defined but not used
vzlist.c:411:1: warning: ‘diskinodes_h_sort_fn’ defined but not used


Hmm... all these *_sort_fn are sort functions generated by means of a few #define statements, and they are used when vzlist needs to sort its output by some column or parameter (vzlist -s). It is very strange that these are not used, because they should be. Let's take a closer look... zOMG! it's a bug!

Apparently, someone was using copy-paste technique* and forgot to change the names of the functions. The bug is, when you ask vzlist to sort its output to, say, dcachesize failcounter values, it sorts it by dcachesize held values instead, because of the wrong sort function used. Such bugs are hard to notice manually, and there are no autotests for vzlist.

* Yes some parts of vzlist is a copy-pasted mess, I am slowly working on untangling it. For example, see my previous cleanup patches (committed back in June 2010):
src/vzlist.c: streamline a few macros
vzlist: put similar print_ functions in a macro
vzlist.c: simplify last_field logic

Morale: sometimes declaring functions as static actually helps!

PS if you see mistakes in this blog post, patches to it are welcome. It's 1am here and I am a bit sleepy.

Tags:

Comments

( 2 comments — Leave a comment )
siraenuhlaalu
Feb. 7th, 2011 10:38 pm (UTC)
In fact, the usage of static functions adds some benefits of clean functional programming into the plain C code.

It is also extremely useful to make code reuse easier and safer in case of compiling static and dynamic libraries.
kot_begemot
Feb. 7th, 2011 11:26 pm (UTC)
One more benefit of making your functions static unless they are called from other files becomes apparent in certain kernels (e.g. ancient Solaris 2.6) where kernel symbol table has a fixed size.
In such kernels, every new module loaded just adds its externally visible symbols (and in particular all non-static functions) to a common pile. This goes on till there is no more space in the name table, so subsequent module load would simply fail.
On a large and complex system there could be (literally) hundreds of modules, some loaded at boot time, other loaded on demand and unloaded when unused.
So, if you have a pig in the mix (i.e. a module that has tons of non-static functions), it can easily wreck a havoc by consuming all the slots in the table and effectively preventing other modules from loading. To add an insult to an injury, such a pig may have literally nothing to do with its victim. Moreover, it may be loaded much earlier than the victim, so it would be all but impossible to correctly diagnose the issue.
( 2 comments — Leave a comment )

Latest Month

July 2016
S M T W T F S
     12
3456789
10111213141516
17181920212223
24252627282930
31      

Comments

Powered by LiveJournal.com
Designed by Tiffany Chow