Code Review

[ScrollLock]

Code Review

טוב, אז כמו שכולנו כבר יודעים, חלק מהקוד של מיקרוסופט "דלף" לרשת ואם כבר אז כבר - למה לא לעשות "Code Review"? מדובר ב-sourceים של Service Pack 1 של Windows 2000 ("ישן משהו"...). קבצי ה-source מכילים חלק מה-kernal של המערכת, ה-shell, ה-registery, ה-debug services של Windows, עוד כמה driverים וכמה קבצי ריצה. בניגוד למה שמקובל לספר על מיקרוסופט (אחד מתכנת ו-10 בודקים), הקוד עצמו די מסודר. יש תיאור של כל קובץ ב-header ובתוך הקוד עצמו יש לא מעט הערות כך שבסה"כ לא מסובך מדי להבין מה קורה בכל פונקציה או פרוצדורה. הקוד בנוי כך ש(כמעט) כל API של Windows הוא קובץ בפני עצמו כך שקל יחסית לנהל את הקוד. רמת הכתיבה היא טובה (יחסית) וניכרת הקפדה על Coding Conventions של הארגון. לפי חלק מההערות בקוד, נראה שנעשה כמה פעמים תהליך של code review וההערות מעידות שישנו עוד מקום לשיפור (ישנם באגים מתועדים ע"י הערות). בסה"כ, למרות הבאגים וכמה דברים שאנחנו היינו ממליצים לשנות ולשפר, הקוד נראה סביר ונראה שאנשי האיכות "עושים את מלאכתם נאמנה". יש מקום לאופטימיזציות אבל יתכן שאלו נעשו בשלב מאוחר יותר. המלצות - ללמוד לשמור על ה-Intellectual property בצורה טובה יותר...

 

[erandd]

עד כמה שונה הקוד הזה מקוד XP לדעתך?

 

[ScrollLock]

נו...

אם תוכל להעביר לי את הקוד של ה- Windows XP אני אוכל להשוות

 

[erandd]

לא הכוונה היא האם הקוד השתנה בהרבה

הרי ידוע שהקרנל נותר די דומה, כמו גם מנגנון הקצאת זכרון ועוד. השאלה היא מה ניתן להסיק מהקוד הזה על מערכות נוכחיות (XPץ 2003 ואולי LONGHORN)

 

[ScrollLock]

והנה כמה commentים שאהבנו במיוחד

Pay attention to the following comments: =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ In the file rle.c: DWORD NEAR _fastcall RgbCompare(RGBQUAD rgb1, RGBQUAD rgb2) { DWORD sum=0; // // lets do some majic shit so the compiler generates "good" code. // =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ In the file webapp.cpp: // HighContrast mode is turned on. This totally fucks our style sheet as most of it will // get ignored. The best we can do is to resize our window so the gigantic fonts will // show correctly. =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ In the file util.cpp // BUGBUG (reinerf) // the fucking alpha cpp compiler seems to fuck up the goddam type "LPITEMIDLIST", so to work // around the fucking peice of shit compiler we pass the last param as an LPVOID instead of a LPITEMIDLIST =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ In the file copy.c: void SendProgressMessage(COPY_STATE *pcs, UINT uMsg, WPARAM wParam, LPARAM lParam) { if (pcs->hwndProgress) SendDlgItemMessage(pcs->hwndProgress, IDD_PROBAR, uMsg, wParam, lParam); } // see if this file is loaded by kernel, thus something we don't // want to fuck with. // =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ verinfo.h: !!!!!!!IF YOU CHANGE TABS SPACES, YOU WILL BE KILLED !!!!!!!!!!!!!!DOING SO FUCKS THE BUILD PROCESS!!!!!!!!!!!!!!!!“ =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ In the file util.h: // // transport didn't load for one reason or another. // wait here until user changes the transport, then try again. // if (pszDll_Tl_NameArgs) { // The user fucked up free(pszDll_Tl_NameArgs); pszDll_Tl_NameArgs = NULL; delete pCTl; } =+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+

 

[Rשף]

גם בקוד שלנו יש לא מעט כאלה

רוצה כמה ? /*fucking default for (developer name remove)*/ /* Level 30th decleration (don't look down !) */ לפונקציה רקורסיבית מיפלצתית