|
Too bad you can't find the guy who did that. Who knows what other gems he has laying around.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
|
KarstenK wrote: I think you find more of these eye-candy if you go for it.
I agree and that was the point of my post earlier 
I would suspect the developer probably did such a thing as a coding standard of their own and the chances of more gems like that are probably bound to be found.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
This horror using GCC for AVR.
static void apicall( void ) __attribute__ ((noinline));
static void apicall( void )
{
asm volatile("ldi r22, %0" :: "M" ((FLASHEND>>1)&0xFF));
asm volatile("push r22");
asm volatile("ldi r22, %0" :: "M" ((FLASHEND>>9)&0xFF));
asm volatile("push r22");
#if( FLASHEND > 0x1FFFF )
asm volatile("ldi r22, %0" :: "M" ((FLASHEND>>17)&0xFF));
asm volatile("push r22");
#endif
asm volatile("ldi r22, %0" :: "M" (API_PROG_PAGE));
return;
}
unsigned char copy_flash( void *src, void *dst, unsigned char dst_hi )
{
unsigned char i;
if( (unsigned int)dst & (SPM_PAGESIZE-1))
return API_ERR_PAGE;
asm volatile("movw r26, %0" :: "r" (src));
asm volatile("movw r30, %0" :: "r" (dst));
asm volatile("mov r21, %0" :: "r" (dst_hi));
apicall();
asm volatile("clr r1");
asm volatile("mov %0, r22" : "=r" (i));
return i;
}
Why push a address on the stack and using the compiler emitted ret instruction instead of using a call instruction?
Assuming CPU registers remain unchanged between asm statements: bad
Changing CPU registers in assembler without informing compiler: worse
Why not write the subroutine to use the C calling convention?
|
|
|
|
|
What can I say? Kids these days...
cheers,
Chris Maunder
CodeProject.com : C++ MVP
|
|
|
|
|
Looks like an attempt to do a tail-call optimisation, so that the subroutine call returns directly to our caller (i.e. copy_flash) rather than to apicall() itself. It's probably an attempt to save a little code space.
This can backfire though: modern processors do a lot of branch prediction, and unbalancing the call/return stack will screw up the branch predictor and make your code run slower. I know Raymond Chen wrote about this but can't find the link.
I would go for a direct jump to the API entry point. Perhaps the processor doesn't have a readily-accessible jump instruction? I'd be astonished if it didn't, though.
As for changing CPU registers, the Application Binary Interface (ABI) for the system will state which registers are volatile - can be changed by a function without saving it - and which are non-volatile - must be saved by the function before being used. A function must save all the volatile registers it's used before making a call, because the called function might trash them.
I agree with you on the weird manual call to apicall() though.
DoEvents: Generating unexpected recursion since 1991
|
|
|
|
|
This processor does have a jump instruction, but putting everything in copy_flash would be better. And the assumption the the return address is at the top of the stack breaks when optimisation is not enabled.
What registers can be changed by a function is irrelevant here. The question is what inline assembler is allowed to do, whilst Visual C++ will parse inline assembler to determine register usage, GCC will not. If one of the registers has been allocated for another purpose by the compiler, it will break.
Neither Visual C++ or GCC guarantees that registers will remain unchanged between consecutive asm statements. If profiling is enabled this code will break.
|
|
|
|
|
Timothy Baldwin wrote: Why not write the subroutine to use the C calling convention?
Some microcontrollers have unchangeable routines in ROM to perform various functions like writing to flash memory. Some of them require that such routines be used when writing to flash because (1) none of the RAM in the system supports code execution, and (2) while a write to flash is taking place, none of the flash memory can be used for any purpose including program execution. The register and calling conventions for such routines are what they are, and there is no way a programmer or compiler author can change them.
Sometimes microcontrollers require some really nasty coding tricks to make things work in practical fashion. When I use such tricks I make sure to thoroughly document what I'm doing, and I avoid using such tricks purely for the sake of "looking impressive". On the other hand, if an interrupt routine is going to be executing 10,000 times per second on a micro which runs 1,000,000 instructions per second, having clear readable code which takes 30 cycles longer than necessary is not as good as having code which is tricky and hard to read, but runs 50 cycles faster.
|
|
|
|
|
Hi Coders,
Here's a code which I wrote few days ago. When I run it it caused my pc hanged. I had to restart my pc for it.
I was creating a windows service few days ago. In that, I put a "Timer Control". And according to it's ticks, I wanted to run my process.
And, for checking purpose by mistake I wrote following Process.Start instead of EventLog.WriteEntry in my code:
private void onTimerEvent(object sender,System.Timers.ElapsedEventArgs e)
{
Process.Start("notepad.exe");
}
It was ok till now but then I put Timer Duration to 100 milisconds!!!!!!!
When I started my Service and Bllllaaaassdsstttt!!!!!!!
My screen was full of NotePads.... n kept comming every 100 miliseconds!!!! until I restarted.
Anand Desai
Developer
Atharva Infotech
|
|
|
|
|
Anand Desai wrote: My screen was full of NotePads.... n kept comming every 100 miliseconds!!!! until I restarted.
LOL 
Anand Desai wrote: Process.Start("nodepad.exe");
Funny, I was expecting a FileNotFoundException
|
|
|
|
|
Noways.... try it. if u dont give path in "Process.Start", it itself take the path of Windows like "C:\\Windows".
This wont give "FileNotFoundException"
Anand Desai
Developer
Atharva Infotech
|
|
|
|
|
... you spelt Notepad wrong...
In your code snippet, it says: "nodepad.exe".
|
|
|
|
|
Ohhh... sorry but that's typing mistake in this site. in my project I actually wrote notepad.exe
Anand Desai
Developer
Atharva Infotech
|
|
|
|
|
#define FILE_NOT_FOUND "notepad.exe"
Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots.
-- Robert Royall
|
|
|
|
|
|
Good choice
"The shortest distance between two points is under construction"
-Noelie ALtito
|
|
|
|
|
Oops 
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
|
|
|
|
|
|
Anand Desai wrote: My screen was full of NotePads.... n kept comming every 100 miliseconds!!!! until I restarted.
 It's the revenge of the notepads because you've been spending too much time with your insert dev env name here.
|
|
|
|
|
Get it an expensive perfume or scrap-booking kit to shut it up.
He who asks a question is a fool for five minutes. He who does not ask a question remains a fool forever. [Chineese Proverb]
Jonathan C Dickinson (C# Software Engineer)
|
|
|
|
|
Could've been worse:
private void onTimerEvent(object sender,System.Timers.ElapsedEventArgs e){
Process.Start("devenv.exe");
}
Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan
Portland, Oregon, USA
|
|
|
|
|
No it can go worse far
If I put that this windows service StartMode to "Automatic":
Just imagine, when ever I start my pc, my blasting windows service also get started automatically n starts throwing notepads to my desktop every 100 miliseconds!!!...untill I restart.. and loop continue........ DeadLock
Hey, This seems simple virus, insn't it?
Anand Desai
Developer
Atharva Infotech
|
|
|
|
|
Yeah, don't get any ideas.
Kyosa Jamie Nordmeyer - Taekwondo Yi (2nd) Dan
Portland, Oregon, USA
|
|
|
|
|
Surely running in safe mode would give you the chance to correct this
|
|
|
|
|
I inherited a Borland Win32 application that has over 60 global variables interspersed throughout it. Many of the globals are multilevel struct instantiations.
Nearly every class definition has a line similar to this after it: "extern PACKAGE TTree *Tree;" The actual declarations of these variables are either in the code for the "about page", above the program's "main" function, or in "globals.cpp"
There is a file called "structs.h" which contains the definitions for every struct used in the application. structs that have to do with the database, visual controls, file contents, etc are all in the same file. The file is 6,000 lines long.
There is a file called "globals.h" that is 3,000 lines long that is nothing but #defines. The #defines are related to, you guessed it: database, individual visual controls, different file formats, and other processing specific code. The file is included in every .cpp file. Very few of the #defines have meaning in more two .cpp files.
8 of the 28 .cpp files in the project are over 10,000 lines long. The longest is 14,433 lines. Most of the code in these mammoths has nothing to do with what the file name would have you think they are doing.
Instead of using one of the hundred different XML processing libraries out there, this guy decides to write his own in order to make a way of saving data. The produced files aren't even valid XML so I can't replace it with a more efficient XML processor. I have to maintain his version of XML syntax for backward compatibility.
There isn't a single pass by reference in the entire application. It is all pointers.
There is no function polymorphism being performed. There are a ton of functions with parameters that specify the kind of data being handled with switch-cases that handle different kinds of data.
Every struct declaration looks like this: "struct CALIB_STRUCT Calib;" Always all caps, always has the "struct" keyword, and always ends with "_STRUCT".
There is no real naming scheme. Sometimes constants are all caps, sometimes they aren't, sometimes they are structs.
He declares every variable that could possibly be used by the function at the top of the function. This includes those huge structs that never get used. Get this guy a copy of the C++ standard please! Many functions are over 500 lines long. Thank goodness for split-screened editors.
The only container that is used are statically defined arrays (usually of structs) even when there is no way to know how many elements will be in it. He just makes it big and forgets about it.
There is no class inheritance, encapsulation, polymorphism, or specialization for different kinds of data. Just dozens of functions with huge multilevel switch-case statements to handle the "special" cases of each type of data.
Every function, I mean every single function, has something similar to this as its first line:
AnsiString errorHeader = "Showing Probe Calibration Error: ";
errorHeader += "Function Where Error Occurred: ";
errorHeader += "TfrmConfig::SaveCalData(); Type of error: ";
Every function, I mean every single function, has this exact code at the end (misc is a global of type TMisc):
} // end catch
catch(Exception &e)
{
errorMsg = errorHeader;
errorMsg += e.ClassName();
errorMsg += " Exception; Error message: " + e.Message;
if (Misc)
Misc->LogSystemError(errorMsg);
error = EI_KNOWN_EXCEPTION;
}
catch(...)
{
errorMsg = errorHeader + "Unknown Error";
if (Misc)
{
Misc->LogSystemError(errorMsg);
}
error = EI_UNKNOWN_EXCEPTION;
}
return error;
There is no other exception handling being performed.
The list goes on. These are just the things I can easily describe here. Overall, this guy just loves to copy and paste code. Apparently making a robust function or class specialization is just too much work.
In debugging and running the code, I find that along a single execution path functions may be needlessly called multiple times to fill visual controls with statically defined data. I am seriously terrified to run a code profiler on it. This guy wrote another similar size app here that has also landed in my lap.
Did I mention that this guy teaches at ITT Tech? He no longer works here (I replaced him). Oddly enough, he wasn't let go for incompetence.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
hide that from the boss..