There are 320 instances in the OpenSim code where floating point numbers are compared using the equality operator. This seems to be an elementary mistake since these comparisons are susceptible to rounding errors.

For example, the m_DayTimeSunHourScale variable is compared with 0.5:

                if (m_DayTimeSunHourScale != 0.5f)

a more portable solution would be to change the code so that the difference is compared to an error threshold. In this case:

                if (Math.Abs(m_DayTimeSunHourScale - 0.5f) > 0)

would be a better option.

78 empty catch blocks.

                        try
                        {
                            lock (m_syncRoot)
                                m_priorityQueue.Delete(imgrequest.PriorityQueueHandle);
                        }
                        catch (Exception)
                        {
                        }

613 instances are in the current OpenSim development branch where a null-reference exception can occur. A good example thereof can be found in Caps.cs:

                        uint port = (MainServer.Instance == null) ? 0 : MainServer.Instance.Port;
                        string protocol = "http";
 
                        if (MainServer.Instance.UseSSL)
                        {
                            hostName = MainServer.Instance.SSLCommonName;
                            port = MainServer.Instance.SSLPort;
                            protocol = "https";
                        }

where "MainServer.Instance" is first checked to be null, sets the port to 0 yet the following if-clause will yield a null reference exception in case MainServer.Instance is indeed null.

In HGSuitcaseInventoryService.cs, the following snippet is supposed to check whether an avatar has a suitcase folder (used for hyperjumping between grids). If the avatar does not have a suitcase folder (checked with suitcase == null), then a new suitcase folder is created:

            if (suitcase == null)
            {
                m_log.DebugFormat("[HG SUITCASE INVENTORY SERVICE]: User {0} does not have a Suitcase folder. Creating it...", principalID);
                // make one, and let's add it to the user's inventory as a direct child of the root folder
                // In the DB we tag it as type 100, but we use -1 (Unknown) outside
                suitcase = CreateFolder(principalID, root.folderID, 100, "My Suitcase");
                if (suitcase == null)
                    m_log.ErrorFormat("[HG SUITCASE INVENTORY SERVICE]: Unable to create suitcase folder");
                m_Database.StoreFolder(suitcase);
 
                // Create System folders
                CreateSystemFolders(principalID, suitcase.folderID);
            }
 
            SetAsNormalFolder(suitcase);
 
            return ConvertToOpenSim(suitcase);

This is fine, but after attempting to create the My Suitcase folder, if the call to CreateFolder returns null, then an error message is logged. However, regardless whether suitcase was null, the code proceeds to store it anyway, possibly calling StoreFolder on a null reference and then does the same again for CreateSystemFolders, SetAsNormalFolder and ConvertToOpenSim.

Patch submitted to mantis.

In RezAttachments, agentId is assigned (UUID)null in case sp.ControllingClient == null:

                catch (Exception e)
                {
                    UUID agentId = (sp.ControllingClient == null) ? (UUID)null : sp.ControllingClient.AgentId;
                    m_log.ErrorFormat("[ATTACHMENTS MODULE]: Unable to rez attachment with itemID {0}, assetID {1}, point {2} for {3}: {4}\n{5}",
                        attach.ItemID, attach.AssetID, attachmentPt, agentId, e.Message, e.StackTrace);
                }

This is then used just in ErrorFormat call. Casting null to UUID breaks the Visual Studio compiler and regardless of the case the ErrorFormat function logs UUID.Zero in case sp.ControllingClient == null. The former can be addressed by using the default keyword instead of casting to null while the latter could be a solution-wide fix to express an "unknown UUID".

Patch submitted to mantis.

3395 classes, methods, constructors, delegates, properties, fields, etc… Are never used, some of them being skelleton objects, for example LLSDMapLayerResponse:

namespace OpenSim.Framework.Capabilities
{
    [LLSDType("MAP")]
    public class LLSDMapLayerResponse
    {
        public LLSDMapRequest AgentData = new LLSDMapRequest();
        public OSDArray LayerData = new OSDArray();
 
        public LLSDMapLayerResponse()
        {
        }
    }
}

String.IsNullOrEmpty can be used instead of various checks, such as:

if (estateName != null && estateName != "")
if (proxyexcepts != null && proxyexcepts.Length > 0)
if ((Channel != null) && (Channel != ""))

there are 45 other occurrences of the same type of checks in the OpenSim development branch.

String.IsNullOrEmpty has been there since .NET 2.0 and it provides a better (unambiguous) way to check whether strings are empty or null. There may be circumstances where a string is empty but not null, in which case not using String.IsNullOrEmpty potentially leads to null reference exceptions.

Patch submitted to mantis.