Eric Pouech | e86389e | 2004-10-26 22:45:47 +0000 | [diff] [blame] | 1 | <chapter id="codingpractice"> |
| 2 | <title>Coding Practice</title> |
| 3 | |
| 4 | <para> |
| 5 | This chapter describes the relevant coding practices in Wine, |
| 6 | that you should be aware of before doing any serious development |
| 7 | in Wine. |
| 8 | </para> |
| 9 | <sect1 id="patch-format"> |
| 10 | <title>Patch Format</title> |
| 11 | |
| 12 | <para> |
| 13 | Patches are submitted via email to the Wine patches mailing |
| 14 | list, <email>wine-patches@winehq.org</email>. Your patch |
| 15 | should include: |
| 16 | </para> |
| 17 | |
| 18 | <itemizedlist> |
| 19 | <listitem> |
| 20 | <para> |
| 21 | A meaningful subject (very short description of patch) |
| 22 | </para> |
| 23 | </listitem> |
| 24 | <listitem> |
| 25 | <para> |
| 26 | A long (paragraph) description of what was wrong and what |
| 27 | is now better. (recommended) |
| 28 | </para> |
| 29 | </listitem> |
| 30 | <listitem> |
| 31 | <para> |
| 32 | A change log entry (short description of what was |
| 33 | changed). |
| 34 | </para> |
| 35 | </listitem> |
| 36 | <listitem> |
| 37 | <para> |
| 38 | The patch in <command>diff -u</command> format |
| 39 | </para> |
| 40 | </listitem> |
| 41 | </itemizedlist> |
| 42 | |
| 43 | <para></para> |
| 44 | |
| 45 | <para> |
| 46 | <command>cvs diff -u</command> works great for the common case |
| 47 | where a file is edited. However, if you add or remove a file |
| 48 | <command>cvs diff</command> will not report that correctly so |
| 49 | make sure you explicitly take care of this rare case. |
| 50 | </para> |
| 51 | <para> |
| 52 | For additions simply include them by appending the |
| 53 | <command>diff -u /dev/null /my/new/file</command> output of |
| 54 | them to any <command>cvs diff -u</command> output you may |
| 55 | have. Alternatively, use <command>diff -Nu olddir/ |
| 56 | newdir/</command> in case of multiple new files to add. |
| 57 | </para> |
| 58 | <para> |
| 59 | For removals, clearly list the files in the description of the |
| 60 | patch. |
| 61 | </para> |
| 62 | <para> |
| 63 | Since wine is constantly changing due to development it is |
| 64 | strongly recommended that you use cvs for patches, if you |
| 65 | cannot use cvs for some reason, you can submit patches against |
| 66 | the latest tarball. To do this make a copy of the files that |
| 67 | you will be modifying and <command>diff -u</command> against |
| 68 | the old file. I.E. |
| 69 | </para> |
| 70 | <screen> |
| 71 | diff -u file.old file.c > file.txt |
| 72 | </screen> |
| 73 | </sect1> |
| 74 | |
| 75 | <sect1 id="Style-notes"> |
| 76 | <title>Some notes about style</title> |
| 77 | |
| 78 | <para> |
| 79 | There are a few conventions that about coding style that have |
| 80 | been adopted over the years of development. The rational for |
| 81 | these <quote>rules</quote> is explained for each one. |
| 82 | </para> |
| 83 | <itemizedlist> |
| 84 | <listitem> |
| 85 | <para> |
| 86 | No HTML mail, since patches should be in-lined and HTML |
| 87 | turns the patch into garbage. Also it is considered bad |
| 88 | etiquette as it uglifies the message, and is not viewable |
| 89 | by many of the subscribers. |
| 90 | </para> |
| 91 | </listitem> |
| 92 | <listitem> |
| 93 | <para> |
| 94 | Only one change set per patch. Patches should address only |
| 95 | one bug/problem at a time. If a lot of changes need to be |
| 96 | made then it is preferred to break it into a series of |
| 97 | patches. This makes it easier to find regressions. |
| 98 | </para> |
| 99 | </listitem> |
| 100 | <listitem> |
| 101 | <para> |
| 102 | Tabs are not forbidden but discouraged. A tab is defined |
| 103 | as 8 characters and the usual amount of indentation is 4 |
| 104 | characters. |
| 105 | </para> |
| 106 | </listitem> |
| 107 | <listitem> |
| 108 | <para> |
| 109 | C++ style comments are discouraged since some compilers |
| 110 | choke on them. |
| 111 | </para> |
| 112 | </listitem> |
| 113 | <listitem> |
| 114 | <para> |
| 115 | Commenting out a block of code is usually done by |
| 116 | enclosing it in <command>#if 0 ... #endif</command> |
| 117 | Statements. For example. |
| 118 | </para> |
| 119 | <screen> |
| 120 | /* note about reason for commenting block */ |
| 121 | #if 0 |
| 122 | code |
| 123 | code /* comments */ |
| 124 | code |
| 125 | #endif |
| 126 | </screen> |
| 127 | <para> |
| 128 | The reason for using this method is that it does not |
| 129 | require that you edit comments that may be inside the |
| 130 | block of code. |
| 131 | </para> |
| 132 | </listitem> |
| 133 | <listitem> |
| 134 | <para> |
| 135 | Patches should be in-lined (if you can configure your |
| 136 | email client to not wrap lines), or attached as plain text |
| 137 | attachments so they can be read inline. This may mean some |
| 138 | more work for you. However it allows others to review your |
| 139 | patch easily and decreases the chances of it being |
| 140 | overlooked or forgotten. |
| 141 | </para> |
| 142 | </listitem> |
| 143 | <listitem> |
| 144 | <para> |
| 145 | Code is usually limited to 80 columns. This helps prevent |
| 146 | mailers mangling patches by line wrap. Also it generally |
| 147 | makes code easier to read. |
| 148 | </para> |
| 149 | </listitem> |
| 150 | <listitem> |
| 151 | <para> |
| 152 | If the patch fixes a bug in Bugzilla please provide a link |
| 153 | to the bug in the comments of the patch. This will make it |
| 154 | easier for the maintainers of Bugzilla. |
| 155 | </para> |
| 156 | </listitem> |
| 157 | </itemizedlist> |
| 158 | <sect2 id="Inline-Attachments-with-OE"> |
| 159 | <title>Inline attachments with Outlook Express</title> |
| 160 | <para> |
| 161 | Outlook Express is notorious for mangling |
| 162 | attachments. Giving the patch a <filename>.txt</filename> |
| 163 | extension and attaching will solve the problem for most |
| 164 | mailers including Outlook. Also, there is a way to enable |
| 165 | Outlook Express send <filename>.diff</filename> |
| 166 | attachments. |
| 167 | </para> |
| 168 | <para> |
| 169 | You need following two things to make it work. |
| 170 | </para> |
| 171 | <orderedlist> |
| 172 | <listitem> |
| 173 | <para> |
| 174 | Make sure that <filename>.diff</filename> files have |
| 175 | \r\n line ends, because if OE detects that there is no |
| 176 | \r\n line endings it switches to quoted-printable format |
| 177 | attachments. |
| 178 | </para> |
| 179 | </listitem> |
| 180 | <listitem> |
| 181 | <para> |
| 182 | Using regedit add key "Content Type" |
| 183 | with value "text/plain" to the |
| 184 | <filename>.diff</filename> extension under |
| 185 | HKEY_CLASSES_ROOT (same as for <filename>.txt</filename> |
| 186 | extension). This tells OE to use |
| 187 | Content-Type: text/plain instead of |
| 188 | application/octet-stream. |
| 189 | </para> |
| 190 | </listitem> |
| 191 | </orderedlist> |
| 192 | <para> |
| 193 | Item #1 is important. After you hit "Send" button, go to |
| 194 | "Outbox" and using "Properties" verify the message source to |
| 195 | make sure that the mail has correct format. You might want |
| 196 | to send several test emails to yourself too. |
| 197 | </para> |
| 198 | </sect2> |
| 199 | <sect2 id="Alexandre-Bottom-Line"> |
| 200 | <title>Alexandre's Bottom Line</title> |
| 201 | <para> |
| 202 | <quote>The basic rules are: no attachments, no MIME crap, no |
| 203 | line wrapping, a single patch per mail. Basically if I can't |
| 204 | do <command>"cat raw_mail | patch -p0"</command> it's in the |
| 205 | wrong format.</quote> |
| 206 | </para> |
| 207 | </sect2> |
| 208 | </sect1> |
| 209 | |
| 210 | <sect1 id="patch-quality"> |
| 211 | <title>Quality Assurance</title> |
| 212 | |
| 213 | <para> |
| 214 | (Or, "How do I get Alexandre to apply my patch quickly so I |
| 215 | can build on it and it will not go stale?") |
| 216 | </para> |
| 217 | <para> |
| 218 | Make sure your patch applies to the current CVS head |
| 219 | revisions. If a bunch of patches are committed to CVS that may |
| 220 | affect whether your patch will apply cleanly then verify that |
| 221 | your patch does apply! <command>cvs update</command> is your |
| 222 | friend! |
| 223 | </para> |
| 224 | <para> |
| 225 | Save yourself some embarrassment and run your patched code |
| 226 | against more than just your current test example. Experience |
| 227 | will tell you how much effort to apply here. If there are |
| 228 | any conformance tests for the code you're working on, run them |
| 229 | and make sure they still pass after your patch is applied. Running |
| 230 | tests can be done by running <command>make test</command>. You may |
| 231 | need to run <command>make testclean</command> to undo the results |
| 232 | of a previous test run. See the <quote>testing</quote> guide for |
| 233 | more details on Wine's conformance tests. |
| 234 | </para> |
| 235 | |
| 236 | </sect1> |
| 237 | <sect1 id="porting"> |
| 238 | <title>Porting Wine to new Platforms</title> |
| 239 | <para> |
| 240 | This document provides a few tips on porting Wine to your |
| 241 | favorite (UNIX-based) operating system. |
| 242 | </para> |
| 243 | |
| 244 | <sect2> |
| 245 | <title> |
| 246 | Why <symbol>#ifdef MyOS</symbol> is probably a mistake. |
| 247 | </title> |
| 248 | |
| 249 | <para> |
| 250 | Operating systems change. Maybe yours doesn't have the |
| 251 | <filename>foo.h</filename> header, but maybe a future |
| 252 | version will have it. If you want to <symbol>#include |
| 253 | <foo.h></symbol>, it doesn't matter what operating |
| 254 | system you are using; it only matters whether |
| 255 | <filename>foo.h</filename> is there. |
| 256 | </para> |
| 257 | <para> |
| 258 | Furthermore, operating systems change names or "fork" into |
| 259 | several ones. An <symbol>#ifdef MyOs</symbol> will break |
| 260 | over time. |
| 261 | </para> |
| 262 | <para> |
| 263 | If you use the feature of <command>autoconf</command> -- the |
| 264 | Gnu auto-configuration utility -- wisely, you will help |
| 265 | future porters automatically because your changes will test |
| 266 | for <emphasis>features</emphasis>, not names of operating |
| 267 | systems. A feature can be many things: |
| 268 | </para> |
| 269 | |
| 270 | <itemizedlist> |
| 271 | <listitem> |
| 272 | <para> |
| 273 | existence of a header file |
| 274 | </para> |
| 275 | </listitem> |
| 276 | <listitem> |
| 277 | <para> |
| 278 | existence of a library function |
| 279 | </para> |
| 280 | </listitem> |
| 281 | <listitem> |
| 282 | <para> |
| 283 | existence of libraries |
| 284 | </para> |
| 285 | </listitem> |
| 286 | <listitem> |
| 287 | <para> |
| 288 | bugs in header files, library functions, the compiler, ... |
| 289 | </para> |
| 290 | </listitem> |
| 291 | </itemizedlist> |
| 292 | <para> |
| 293 | You will need Gnu Autoconf, which you can get from your |
| 294 | friendly Gnu mirror. This program takes Wine's |
| 295 | <filename>configure.ac</filename> file and produces a |
| 296 | <filename>configure</filename> shell script that users use |
| 297 | to configure Wine to their system. |
| 298 | </para> |
| 299 | <para> |
| 300 | There <emphasis>are</emphasis> exceptions to the "avoid |
| 301 | <symbol>#ifdef MyOS</symbol>" rule. Wine, for example, needs |
| 302 | the internals of the signal stack -- that cannot easily be |
| 303 | described in terms of features. Moreover, you can not use |
| 304 | <filename>autoconf</filename>'s <symbol>HAVE_*</symbol> |
| 305 | symbols in Wine's headers, as these may be used by Winelib |
| 306 | users who may not be using a <filename>configure</filename> |
| 307 | script. |
| 308 | </para> |
| 309 | <para> |
| 310 | Let's now turn to specific porting problems and how to solve |
| 311 | them. |
| 312 | </para> |
| 313 | </sect2> |
| 314 | |
| 315 | <sect2> |
| 316 | <title> |
| 317 | MyOS doesn't have the <filename>foo.h</filename> header! |
| 318 | </title> |
| 319 | |
| 320 | <para> |
| 321 | This first step is to make <command>autoconf</command> check |
| 322 | for this header. In <filename>configure.in</filename> you |
| 323 | add a segment like this in the section that checks for |
| 324 | header files (search for "header files"): |
| 325 | </para> |
| 326 | <programlisting> |
| 327 | AC_CHECK_HEADER(foo.h, AC_DEFINE(HAVE_FOO_H)) |
| 328 | </programlisting> |
| 329 | <para> |
| 330 | If your operating system supports a header file with the |
| 331 | same contents but a different name, say |
| 332 | <filename>bar.h</filename>, add a check for that also. |
| 333 | </para> |
| 334 | <para> |
| 335 | Now you can change |
| 336 | </para> |
| 337 | <programlisting> |
| 338 | #include <foo.h> |
| 339 | </programlisting> |
| 340 | <para> |
| 341 | to |
| 342 | </para> |
| 343 | <programlisting> |
| 344 | #ifdef HAVE_FOO_H |
| 345 | #include <foo.h> |
| 346 | #elif defined (HAVE_BAR_H) |
| 347 | #include <bar.h> |
| 348 | #endif |
| 349 | </programlisting> |
| 350 | <para> |
| 351 | If your system doesn't have a corresponding header file even |
| 352 | though it has the library functions being used, you might |
| 353 | have to add an <symbol>#else</symbol> section to the |
| 354 | conditional. Avoid this if you can. |
| 355 | </para> |
| 356 | <para> |
| 357 | You will also need to add <symbol>#undef HAVE_FOO_H</symbol> |
| 358 | (etc.) to <filename>include/config.h.in</filename> |
| 359 | </para> |
| 360 | <para> |
| 361 | Finish up with <command>make configure</command> and |
| 362 | <command>./configure</command>. |
| 363 | </para> |
| 364 | </sect2> |
| 365 | |
| 366 | <sect2> |
| 367 | <title> |
| 368 | MyOS doesn't have the <function>bar</function> function! |
| 369 | </title> |
| 370 | |
| 371 | <para> |
| 372 | A typical example of this is the <function>memmove</function> |
| 373 | function. To solve this problem you would add |
| 374 | <function>memmove</function> to the list of functions that |
| 375 | <command>autoconf</command> checks for. In |
| 376 | <filename>configure.in</filename> you search for |
| 377 | <function>AC_CHECK_FUNCS</function> and add |
| 378 | <function>memmove</function>. (You will notice that someone |
| 379 | already did this for this particular function.) |
| 380 | </para> |
| 381 | <para> |
| 382 | Secondly, you will also need to add |
| 383 | <symbol>#undef HAVE_BAR</symbol> to |
| 384 | <filename>include/config.h.in</filename> |
| 385 | </para> |
| 386 | <para> |
| 387 | The next step depends on the nature of the missing function. |
| 388 | </para> |
| 389 | |
| 390 | <variablelist> |
| 391 | <varlistentry> |
| 392 | <term>Case 1:</term> |
| 393 | <listitem> |
| 394 | <para> |
| 395 | It's easy to write a complete implementation of the |
| 396 | function. (<function>memmove</function> belongs to |
| 397 | this case.) |
| 398 | </para> |
| 399 | <para> |
| 400 | You add your implementation in |
| 401 | <filename>misc/port.c</filename> surrounded by |
| 402 | <symbol>#ifndef HAVE_MEMMOVE</symbol> and |
| 403 | <symbol>#endif</symbol>. |
| 404 | </para> |
| 405 | <para> |
| 406 | You might have to add a prototype for your function. |
| 407 | If so, <filename>include/miscemu.h</filename> might be |
| 408 | the place. Don't forget to protect that definition by |
| 409 | <symbol>#ifndef HAVE_MEMMOVE</symbol> and |
| 410 | <symbol>#endif</symbol> also! |
| 411 | </para> |
| 412 | </listitem> |
| 413 | </varlistentry> |
| 414 | <varlistentry> |
| 415 | <term>Case 2:</term> |
| 416 | <listitem> |
| 417 | <para> |
| 418 | A general implementation is hard, but Wine is only |
| 419 | using a special case. |
| 420 | </para> |
| 421 | <para> |
| 422 | An example is the various <function>wait</function> |
| 423 | calls used in <function>SIGNAL_child</function> from |
| 424 | <filename>loader/signal.c</filename>. Here we have a |
| 425 | multi-branch case on features: |
| 426 | </para> |
| 427 | <programlisting> |
| 428 | #ifdef HAVE_THIS |
| 429 | ... |
| 430 | #elif defined (HAVE_THAT) |
| 431 | ... |
| 432 | #elif defined (HAVE_SOMETHING_ELSE) |
| 433 | ... |
| 434 | #endif |
| 435 | </programlisting> |
| 436 | <para> |
| 437 | Note that this is very different from testing on |
| 438 | operating systems. If a new version of your operating |
| 439 | systems comes out and adds a new function, this code |
| 440 | will magically start using it. |
| 441 | </para> |
| 442 | </listitem> |
| 443 | </varlistentry> |
| 444 | </variablelist> |
| 445 | <para> |
| 446 | Finish up with <command>make configure</command> and |
| 447 | <command>./configure</command>. |
| 448 | </para> |
| 449 | </sect2> |
| 450 | </sect1> |
| 451 | |
| 452 | <sect1 id="adding-languages"> |
| 453 | <title>Adding New Languages</title> |
| 454 | |
| 455 | <para> |
| 456 | This file documents the necessary procedure for adding a new |
| 457 | language to the list of languages that Wine can display system |
| 458 | menus and forms in. Adding new translations is not hard as it |
| 459 | requires no programming knowledge or special skills. |
| 460 | </para> |
| 461 | |
| 462 | <para> |
| 463 | Language dependent resources reside in files |
| 464 | named <filename>somefile_Xx.rc</filename> or |
| 465 | <filename>Xx.rc</filename>, where <literal>Xx</literal> |
| 466 | is your language abbreviation (look for it in |
| 467 | <filename>include/winnls.h</filename>). These are included |
| 468 | in a master file named <filename>somefile.rc</filename> or |
| 469 | <filename>rsrc.rc</filename>, located in the same |
| 470 | directory as the language files. |
| 471 | </para> |
| 472 | |
| 473 | <para> |
| 474 | To add a new language to one of these resources you |
| 475 | need to make a copy of the English resource (located |
| 476 | in the <filename>somefile_En.rc</filename> file) over to |
| 477 | your <filename>somefile_Xx.rc</filename> file, include this |
| 478 | file in the master <filename>somefile.rc</filename> file, |
| 479 | and edit the new file to translate the English text. |
| 480 | You may also need to rearrange some of the controls |
| 481 | to better fit the newly translated strings. Test your changes |
| 482 | to make sure they properly layout on the screen. |
| 483 | </para> |
| 484 | |
| 485 | <para> |
| 486 | In menus, the character "&" means that the next |
| 487 | character will be highlighted and that pressing that |
| 488 | letter will select the item. You should place these |
| 489 | "&" characters suitably for your language, not just |
| 490 | copy the positions from English. In particular, |
| 491 | items within one menu should have different highlighted |
| 492 | letters. |
| 493 | </para> |
| 494 | |
| 495 | <para> |
| 496 | To get a list of the files that need translating, |
| 497 | run the following command in the root of your Wine tree: |
| 498 | <command>find -name "*En.rc"</command>. |
| 499 | </para> |
| 500 | |
| 501 | <para> |
| 502 | When adding a new language, also make sure the parameters |
| 503 | defined in <filename>./dlls/kernel/nls/*.nls</filename> |
| 504 | fit your local habits and language. |
| 505 | </para> |
| 506 | </sect1> |
| 507 | </chapter> |
| 508 | |
| 509 | <!-- Keep this comment at the end of the file |
| 510 | Local variables: |
| 511 | mode: sgml |
| 512 | sgml-parent-document:("wine-devel.sgml" "set" "book" "part" "chapter" "") |
| 513 | End: |
| 514 | --> |