OpenSolaris

You are not signed in. Sign in or register.

RTI nits to avoid

Submitters

  • Security issues
    • Sun has to walk a fine line with security fixes between customers on older releases who need patches and customers running the latest stuff for which the source is available. Sometimes this involves coordinating patch releases with 3rd party organizations and other vendors. Thus we handle each such issue individually; to do so, please coordinate with <scteam at sun dot com>.
  • SCCS Keywords
    • SCCS keywords, where used, must not be expanded in your source code. wx nits will verify this for you.
    • SCCS keywords must not be used as an interface for checking versioning or be output in any way, shape or form. This is required to help with our SCM migration project, which will be moving OpenSolaris based releases away from SCCS. Other options include maintaining the version number manually, leveraging hg tip in a Makefile after mercurial is live, or stop trying to track the version number in the driver and instead leverage existing information like the list of installed packages and mdb commands (::showrev -v and ::objects -v).
    • SCCS keywords, including %I%, must not be used outside file ident strings in Nevada. If you are modifying a file for Nevada that contains SCCS keywords outside an ident string, you are expected to remove them.
  • The following "paperwork items" in the bug (CR) reports should be done:
    • The Description text should contain, surprisingly enough, a description of the problem, which should be at least as detailed as the Synopsis, and preferably much more. Note in particular that "See Comments" is never appropriate. For security issues where there is a temporary need for secrecy until the problem is made public, put something vague then plan on updating it to be specific once the issue is made public. For issues involving proprietary customer information, either redact any proprietary information or move it to the Comments text, and write a high-level description which does not contain proprietary data.
    • The Status field should not be 1-Dispatched. 7-Fix in Progress is usual, though 5-Cause Known, 6-Fix Understood and even 8-Fix Available are fine too.
    • For Defects (as opposed to RFEs), the Introduced in Release/Build and Root Cause (under the More Info tab) fields must be filled in. Note that the Introduced in Release/Build field's value should be that of a marketing release (e.g., s10_01), not that of an update release. Also, if a bug ID exists for the changes(s) which introduced this problem, that/those bug ID(s) should be listed in the See Also field. If the bug has been around "forever", using "solaris_2.0" as an Introduced in Release is acceptable.
    • With respect to the Suggested Fix text, some very helpful suggestions follow, though of course common sense should always be applied:
      • A brief high-level description of the changes is good, especially if the diffs won't fit and need to be attached.
      • Each source module's full path (e.g., usr/src/.../foo.c) should be specified in the Suggested Fix text. Further, this text field should contain the diffs for each affected module. Context diffs (diff -c or diff -u, or sccs diffs -C) are preferred if they will fit. If the diffs don't fit in Bugster, then add a note indicating such and attach them.
      • Note that the diffs should be in a permanent form, such as described above. A URL pointing to a webrev or any other transient data is allowable, but insufficient for a bug report, as the data in the bug report is expected to last, whereas webrevs and other URLs often disappear shortly after putback.
      • The diffs should be pointing the "right" way. webrev and sccs diffs will get this right; using diff from the command line, make sure you do diff old new.
      • When fixing multiple bugs as part of single wad, list the minimal set of diffs whenever possible. Sometimes the fixes are too intertwined to list separately, which is OK, but when that is not the case, listing minimal diffs makes it easier to understand each fix separately.
    • If your changes require corresponding changes in Solaris man pages or documentation, then you should also make sure that:
      • The Fix Affects Documentation field (under the More Info tab in the bug report) should be set to yes to match.
      • File man page and/or documentation bugs as needed, and list those bug IDs in WebRTI.
      • The corresponding man page bug IDs should be listed in the See Also field of the engineering bug report.
      Furthermore, WebRTI requires that man page bugs be in the 7-Fix in Progress state prior to submitting an RTI. The man page group therefore recommends:
      • Filing man page bugs 4 builds aheads for moderately large projects, 2 builds ahead for minor changes to one or more man pages.
      • If your change needs to be integrated in the current build, file the man page bug as soon as you know that your code fix requires man page changes.
      • Include specific instructions regarding how to fix the man page in the man page bug.
      • Cite the associated engineering bug in the man page bug.
      • Notify the man page representative to the C-Team, currently Pattie Levinson, that your man page bug is urgent. She will ensure that the man page receives immediate priority, or that you receive an exemption to putback your code without the man page CR being in the 7-Fix in Progress state.
    • It is inappropriate for the Description text to say nothing more than "See Comments." While proprietary data (Sun confidential or customer confidential) must not be placed in the Description text, as much data as possible should be placed there. In particular, the Description should be at least as illuminating as the Synopsis, and preferably much more so.
    • The Synopsis should be a concise description of the problem, free from references to a specific build or release (unless only that build or release is impacted), and free from spelling and grammatical errors. Most of all, it should make sense. As the CR is investigated and the issue is better understood, the Synopsis should be updated to make sure it remains an accurate synopsis of the issue being addressed. This text often appears later in patch README files and other externally consumable formats.
  • The RTI should also have your `putback -n` in the putback -n section and a pointer to your workspace in the comments section. Note that a webrev URL generally covers both of these. Note also that if you run `putback -n` on the gate machine (which is allowed), the path to your child workspace (which you should include in the RTI) will be sufficiently qualified for an RTI advocate (who will often inspect your workspace) to find it.
  • The list of code reviewers should be individuals who have actually reviewed the code, not an alias for a group of people who were given the opportunity to do so.
  • Other more general nits:
    • No bug IDs in the source code; the sccs delta comments cover that.
    • Each source file should add only a single delta, without "merge turds". In the case where a putback includes multiple bug fixes and/or ARC cases, multiple deltas are allowed, but there should still only be a single delta per bug fix and/or ARC case.
  • Here is an explanation of what the Risk and Impact fields are for:
    • Risk assesses how much confidence that we have that the fix will not introduce other problems, and a component of how easy it would be to avoid them problems if they occur.
    • Impact is somewhat akin to external side-effects: the customer will no longer see the bug, but will notice something else in an adverse way.
      • API changes might stop an application from working.
      • Header file changes may cause code compilation problems. Note that e.g. if you have a private header file and you fix all of the consumers, that does not count as external, so you need not mark Header impact. But header files consumer by other projects generally merit Header impact and updating a header file which is shipped in /usr/include always merits Header impact.
      • Packaging / namespace may remove something they rely on if they have built-in paths
      • Documentation changes can have subtler effects.
      • "Other" includes everything else and we require details in the comment section of the RTI to say what it is. Examples of other effects are:
        • slower
        • uses more memory
        • change in messages not covered by documentation changes
        • causes a flag day
        • requires a coordinated putback in a different consolidation
      An Impact of none is the default case: The bug got fixed is all that the customer can detect. If Impact is not none, it should always be explained in the RTI comments.

Reviewers

  • Naked strcpy(), strcat() and sprintf() calls should be avoided wherever possible; use strlcpy(), strlcat() and snprintf() respectively instead.
  • You should not be listed as a reviewer for your own RTI: the point is that someone else should review your code.

Advocates

  • You can not be the advocate for your own RTI: the point is that someone else should review your work. Furthermore, if you are listed as one of the reviewers, then make sure that there is at least one other reviewer (or two in the builds leading up to a milestone) besides yourself.
  • Workspace checking is highly recommended, as it is easy and quick and has prevented many problems; also, many problems which could have easily been prevented were from workspaces which were not checked.

Sponsors for External Contributors

  • The main thing is to make sure that an SCA (Sun Contributor Agreement) is on file for the contributor. The list of contributor agreements is kept internally; someone on the OpenSolaris team such as Linda Bernal would be good to check with about this.
  • The rest of the "sponsoring nits" are on the Sponsor Tasks page.