[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mozilla-os2
Subject:    WAS Re: Beta 2 - getter_AddRefs usage
From:       Lars Erdmann <lars.erdmann () arcor ! de>
Date:       2015-01-08 11:33:18
Message-ID: Ra6dncJoFOpj9zPJnZ2dnUU7-VmdnZ2d () mozilla ! org
[Download RAW message or body]

Hallo David,

I had a look at the various places where "NS_NewNativeLocalFile" is 
called (and it's MANY places).

What strikes me as odd is that most of the time it is called in this way 
(example):
nsCOMPtr<nsIFile> overrideDir;
rv = NS_NewNativeLocalFile(path, false, getter_AddRefs(overrideDir));



where sometimes it is called in this way (For example in 
nslocalFileOS2.cpp):

nsIFile *file;
nsresult rv = NS_NewNativeLocalFile(nsDependentCString(drive),false,&file);



There is a comment regarding "getter_AddRefs" and the "already_AddRefed" 
type that it returns:
<quote>
You might want to use this as a return type from a function
that produces an already |AddRef|ed pointer as a result.
</quote>



Given that NS_NewNativeLocalFile looks this way:
NS_NewNativeLocalFile(const nsACString &path, bool followLinks, nsIFile* 
*result)
{
    NS_ENSURE_ARG_POINTER(result);
    *result = NULL;

    if (!path.IsEmpty()) {
       nsLocalFile* file = new nsLocalFile();
       if (file == nullptr)
         return NS_ERROR_OUT_OF_MEMORY;
       NS_ADDREF(file);

       nsresult rv = file->InitWithNativePath(path);
       if (NS_FAILED(rv)) {
          NS_RELEASE(file);
          return rv;
       }
    }
    *result = file;
    return NS_OK;
}

I'd say that the first form is correct and the second one is not or at 
least questionable. That's because "NS_NewNativeLocalFile" creates a 
reference (to the newly created file) with NS_ADDREF which it basically 
returns in its third argument.

Let's see what the "safety updates" to NS_NewNativeLocalFile will do.


Lars

_______________________________________________
dev-ports-os2 mailing list
dev-ports-os2@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-ports-os2
[prev in list] [next in list] [prev in thread] [next in thread]