[REF][SUGGESTION] clean up your codes for OC/UV and old kernel codes if possible - Galaxy S II Original Android Development

hi all,
first of all, i would like to say that i'm no expert in c, and this post is not intended to blame anyone but want to raise this concern only.
since i started to mod i9000's kernel source code, i found some unneeded conditional statements in the original source and modded OC/UV sections which can be avoided very easily and thus consuming less resources.
let's take a look in this example from the original source:
Code:
for (i = 0; i < LOOP_CNT; i++) {
tmp = __raw_readb(iem_base + S5PV310_APC_DBG_DLYCODE);
sum_result += tmp;
hpm[i] = tmp;
}
for (i = 0; i < LOOP_CNT; i++)
printk(KERN_INFO "ASV : hpm[%d] = %d value\n", i, hpm[i]);
the second for loop is definitely unnecessary, and it should be changed to something like:
Code:
for (i = 0; i < LOOP_CNT; i++) {
tmp = __raw_readb(iem_base + S5PV310_APC_DBG_DLYCODE);
sum_result += tmp;
hpm[i] = tmp;
printk(KERN_INFO "ASV : hpm[%d] = %d value\n", i, hpm[i]);
}
let's take a look at another extreme case, taken from the OC/UV section:
Code:
if (index < L6)
{
if (old_index == L0)
index = L1;
if (old_index == L1)
index = L2;
if (old_index == L2)
index = L3;
if (old_index == L3)
index = L4;
if (old_index == L4)
index = L5;
if (old_index == L5)
index = L6;
}
it is obvious that, if old_index=L0, it has to go thru the remaining 5 if statements which shouldnt be. isnt it be nice and more efficient to chg it to something like below or use switch case instead?
Code:
if (index < L6)
{
if (old_index == L0)
index = L1;
else if (old_index == L1)
index = L2;
else if (old_index == L2)
index = L3;
else if (old_index == L3)
index = L4;
...
...
...
...
}
since we all aim at producing better and faster kernels, i think this is a good practice to clean up the codes while we r modding
thx for your kind attention

---

and this is the most ridiculous part that i've ever seen in my life, which can be found in I9000 source also. if u have 10 frequencies set, then instead of running the loop 10 times, this will run 50 times in total
arch/arm/mach-s5pv310/cpufreq.c, inside static int s5pv310_update_dvfs_table()
for (i = 1; i < CPUFREQ_LEVEL_END; i++) {
s5pv310_freq_table[i-1].index = s5pv310_lookup_freq_table.index - 1;
s5pv310_freq_table[i-1].frequency = s5pv310_lookup_freq_table.frequency;
printk(KERN_INFO "index = %d, frequency = %d\n",
s5pv310_freq_table[i-1].index, s5pv310_freq_table[i-1].frequency);
}
for (i = 1; i < CPUFREQ_LEVEL_END; i++) {
s5pv310_volt_table[i-1].index = s5pv310_lookup_volt_table.index - 1;
s5pv310_volt_table[i-1].arm_volt = s5pv310_lookup_volt_table.arm_volt;
printk(KERN_INFO "index = %d, arm_volt = %d\n",
s5pv310_volt_table[i-1].index, s5pv310_volt_table[i-1].arm_volt);
}
for (i = 1; i < CPUFREQ_LEVEL_END; i++) {
s5pv310_apll_pms_table[i-1] = s5pv310_lookup_apll_pms_table;
printk(KERN_INFO "apll pms_table = 0x%08x\n", s5pv310_apll_pms_table[i-1]);
}
for (i = 1; i < CPUFREQ_LEVEL_END; i++) {
for (j = 0; j < 7; j++) {
clkdiv_cpu0[i-1][j] = clkdiv_cpu0_lookup[j];
printk("%d, ", clkdiv_cpu0[i-1][j]);
}
printk("\n");
}
for (i = 1; i < CPUFREQ_LEVEL_END; i++) {
for (j = 0; j < 2; j++) {
clkdiv_cpu1[i-1][j] = clkdiv_cpu1_lookup[j];
printk("%d, ", clkdiv_cpu1[i-1][j]);
}
printk("\n");
}

---

Interesting...
Sent from my GT-I9100 using XDA Premium.

---

This might be right. My C is bad, but some OC/UV patches look really quite 'whatever works' for me. I dont say its bad - having something is better than nothing.
Its always good if some skilled coders do reviews of such patches.

---

good finding and the samsung programmers are really ............... bad...

---

joecisd said:
good finding and the samsung programmers are really ............... bad...
Click to expand...
Click to collapse
i think it's not samsung's problem, instead, it should be unsolved/overlooked problems brought forward from linux kernel

---

yeaaa Hong Kong
Do you want make a kernel for us?
regards

---

Nice catches
Hope most devs are already taking care of this!

---

avetny said:
yeaaa Hong Kong
Do you want make a kernel for us?
regards
Click to expand...
Click to collapse
sorry, but maybe u don know me coz i'm a newbie here in the i9100 section
as in the i9000 forum, i usually prefer not to release files, except some quick fixes like sms sent time mod, but i'll tell u guys my findings, dirty tricks, and how to instead
sorry about that

---

glad to see that some of you guys already fixed, or started to fix those redundant conditional statements
and ultimately, hope that one day these will be collected as a whole and stored somewhere for everyone like our kernel source code repo here

---

ykk_five said:
glad to see that some of you guys already fixed, or started to fix those redundant conditional statements
and ultimately, hope that one day these will be collected as a whole and stored somewhere for everyone like our kernel source code repo here
Click to expand...
Click to collapse
I go try to clean up my code.
I only have a little C++ skills and no C but I give it a try ...

---

netchip said:
I go try to clean up my code.
I only have a little C++ skills and no C but I give it a try ...
Click to expand...
Click to collapse
maybe if you upload the code to somewhere (for example into a wiki, or pastebin???) we can check it. Personally I've 20years of programming exp in 4 languages, and I'm sure a lot of people like me is already here We can't make the ROM "a lot faster", but basic structural bugs can be wiped out in a short period of time (like the "if else if else if else" example in the first post).
PS: sry for my english

---

I'm also not an expert in C or kernel development, but i'm quite sure, that gcc is smart enough to check that some code is redundant and do that kind of optimizations in compile time (if you enable the optimize flags).
Feel free to give it a try, i'm also curious, but i think the final assembly code will be pretty much similar.
Sent from my GT-I9100 using xda premium

---

killerjohn said:
maybe if you upload the code to somewhere (for example into a wiki, or pastebin???) we can check it. Personally I've 20years of programming exp in 4 languages, and I'm sure a lot of people like me is already here We can't make the ROM "a lot faster", but basic structural bugs can be wiped out in a short period of time (like the "if else if else if else" example in the first post).
PS: sry for my english
Click to expand...
Click to collapse
Okay, I go.do.that
Sent from my GT-I9100

---

netchip said:
I go try to clean up my code.
I only have a little C++ skills and no C but I give it a try ...
Click to expand...
Click to collapse
yeah, i know. i was learning from your source in github last nite
killerjohn said:
maybe if you upload the code to somewhere (for example into a wiki, or pastebin???) we can check it. Personally I've 20years of programming exp in 4 languages, and I'm sure a lot of people like me is already here We can't make the ROM "a lot faster", but basic structural bugs can be wiped out in a short period of time (like the "if else if else if else" example in the first post).
PS: sry for my english
Click to expand...
Click to collapse
that's great. yes, most of them are structural problems actually
Guiper said:
I'm also not an expert in C or kernel development, but i'm quite sure, that gcc is smart enough to check that some code is redundant and do that kind of optimizations in compile time (if you enable the optimize flags).
Click to expand...
Click to collapse
i know gcc is quite smart but i doubt if it can handle the conditional statements automatically if something like:
if (a is a male) then a is a boy
if (a is a female) then a is a girl
so actually, a should be checked once only coz we all know that in our daily life. but sometimes we need a bit more complicated logics like:
if (a contains red) then color = red
if (a contains green) then color =green
if (a contains blue) then color =blue
if (a contains blue & green & blue) then color = gray/black/white (by assuming the portions of red, green and blue are equal)
perhaps this is not a good example, but i just want to point out that not every set of conditions are redundant sometimes and hence gcc may not be able to handle it, i think (i'm not a gcc expert neither, pls correct me if i'm wrong )
Feel free to give it a try, i'm also curious, but i think the final assembly code will be pretty much similar.
Click to expand...
Click to collapse
but i've modded some smali before and found that even a very simple for loop in java can produce a lot of goto statements in the assembly code which can be clean up also