2

In my code I use a method getNumberOfP_ForAdd(). This method opens a file and returns a XMLDocumentWrapper*.

XMLDocumentWrapper* Tab::getNumberOfP_ForAdd()
{
QString defaultName = GuiUtil::getLastPath();
QString fileName = QFileDialog::getOpenFileName(this, "Open " +  displayName + " File",
           defaultName, displayName     + " Files (*." + fileSuffix + ")", 0, 0);

if (fileName.isNull() || fileName.isEmpty()) {
    qDebug() << "Load" << displayName << "aborted.";
    return NULL;
}

GuiUtil::setLastPath(fileName);

// Open file
XMLDocumentWrapper* inputDoc = XMLDocumentWrapper::readFromFile(fileName);
if (inputDoc == NULL) {
    qDebug() << "Load" << displayName << "aborted.";
    return NULL ;
}

return inputDoc;
}

When I try to read a file there are 2 things I check first: Wheather

(fileName.isNull() || fileName.isEmpty())

and

(inputDoc == NULL)

If these statements are true I do

return NULL;

Can I simply return a NULL-Ptr or would I run into problems in doing so? Do I have to free that pointer again?

demonplus
  • 5,613
  • 12
  • 49
  • 68
user3443063
  • 1,455
  • 4
  • 23
  • 37
  • 3
    While perfectly possible, there is a more general remark I would like to make: Is returning a "naked" pointer like that really necessary? This is C++, not C.... – DevSolar Oct 20 '15 at 08:58
  • @DevSolar There are quite often reasons to return NULL-Pointers. If you like to return an item-pointer from a list of items, but that item does not exist, you would return a NULL pointer and check for NULL after the get* Method has been called. This avoids having to call a "itemExists()", with the need of twice looping the list, or having the disadvantage of an overhead for applying an parameter to check validity (Not yet mentioning the overhead of creating a empty item for return) or setting up an exception. – Sebastian Lange Oct 20 '15 at 09:17
  • @SebastianLange: `.end()`? I admit you still have to *check* for the condition, but at least you don't have to think twice about ownership of a pointed-to object.... – DevSolar Oct 20 '15 at 09:20

4 Answers4

6

You can return a NULL pointer.

Do I have to free that pointer again?

delete is only necessary if you allocate space on the free store.

However, if you use C++11 or higher, I recommend the usage of nullptr instead of NULL. In this particular code, it doesn't provide a significant improvement but I consider it good style, also to draw the line between C and C++.

Community
  • 1
  • 1
cadaniluk
  • 15,027
  • 2
  • 39
  • 67
6
  1. You can return a null pointer if your function's return type is a pointer.

  2. If the caller checks the return value and handles the case where the return value is null - then there shouldn't be any problem. if the caller tries to dereference this pointer, then problems do occur.

  3. The fact that the function returns a pointer doesn't necessarily mean the developer needs to free the memory. the pointer could point to anything, including stack and global variables. The developer should look at the function's documentation to make sure what to do with the return value. Anyway, there is no problem in deleting null pointers, but again , one should look if she/he needs to deallocate something at the first place.

David Haim
  • 25,446
  • 3
  • 44
  • 78
  • 1
    If a pointer returned by a function points to something on that function's stack, that's not good, since dereferencing the pointer would be UB. – DevSolar Oct 20 '15 at 08:56
  • 1
    yes, but the function can get, for example, a reference to std::array and return a pointer to one of the elements. this is a case of a stack variable that shouldn't be deleted – David Haim Oct 20 '15 at 08:58
  • 2
    it may return null if it didn't "find" any good element to return. you can't do it with plain reference – David Haim Oct 20 '15 at 09:01
  • 1
    In that case it should return an *iterator* of that array, and `.end()` if it couldn't "find" a good element to return. (Actually, scratch that "reference" comment above. It should *always* return an iterator, since the argument array could be empty. Unless, of course, that would be reason enough for an exception.) There are *very* few arguments to be made for tossing naked pointers around... – DevSolar Oct 20 '15 at 09:10
  • this is arguable, I agree that one shouldn't return raw pointer without thinking about alternatives first, but one may also face a legacy code which does it, or even come across a C library that can only return pointers. also, when dealing with DLLs many exports are plain C types because of no standard ABI. – David Haim Oct 20 '15 at 09:15
  • @DevSolar One thing to think about: When debugging, an `.end()` iterator isn't very useful. You just see the random data that comes after the array in memory. Whereas with a pointer you can easily see it's null. – Emil Laine Oct 20 '15 at 10:56
  • @zenith: I take the burden of checking an iterator against a container's `end_of_storage` instead of `0` any time for the benefits of clear object ownership and being able to plug the function directly into standard algorithms. David has the valid thoughts here -- C compatiblity. Within pure C++, you just shouldn't be looking at naked pointers. – DevSolar Oct 20 '15 at 11:05
  • True, but you wouldn't use naked owning pointers in the first place, and instead use them only for non-owning purposes. Although the point about standard algorithm interoperability makes sense. – Emil Laine Oct 20 '15 at 11:14
5

Can I simply return a NULL-Ptr?

Yes

Would I run into problems in doing so?

Only if you try to call methods on that pointer, that would be undefined behaviour.

Do I have to free that pointer again?

No, using delete or free on a NULL pointer is valid but unnecessary.

TartanLlama
  • 63,752
  • 13
  • 157
  • 193
0

I think that your question in general has been answered, but.... be careful. I know nothing about the XMLDocumentWrapper class. Off hand, I expect that the caller must clean-up (delete) this returned pointer.

Andrew
  • 816
  • 7
  • 15