4

(Using visual studio 2010)

I have a simple returnLength() function for a list, defined simply as

int returnLength() 
{
    if (!next) return 1;
    else return 1 + next->returnLength();
}

next is a pointer to another node in the list. When checking the if statement to check if next is valid, Visual Studio is throwing a runtime error citing Access violation. This error is occurring several calls deep into the recursion tree.

What is the recommended way to check whether a pointer exists?

3
  • 6
    The next member isn't the problem, this is probably null. Check the object reference in the caller. Mar 1, 2011 at 0:51
  • 1
    On the side: What is the length of an empty list, 0 or 1? returnLength() seem to return 1.
    – Arun
    Mar 1, 2011 at 3:11
  • All these people saying that this is probably null are wrong and not paying attention -- this cannot possibly be null at any but the first level of recursion. Rather, this (which was next of the previous node) is some other invalid value, because next of the last node wasn't initialized.
    – Jim Balter
    Feb 25, 2013 at 5:45

7 Answers 7

4

That is a valid way. What's likely happening here is that this is NULL and hence you're getting an access violation trying to read next off of NULL pointer. You need to check for NULL at the callsite to returnLength

3
  • 2
    Very interesting. The runtime error is occurring several layers deep within this recursive call... So it seems like this should be catching itself..
    – pbj
    Mar 1, 2011 at 0:53
  • @pbj: the plague of undefined behavior, anything can happen, so most of the times it looks like it's working fine for a while... Mar 1, 2011 at 7:18
  • Wrong answer. this (which was next from the previous node) is obviously some uninitialized value other than NULL.
    – Jim Balter
    Feb 25, 2013 at 5:46
3

The code you've shown is valid. What is most likely happening is that this function is being called through an invalid this pointer.

2
  • An invalid this pointer shouldn't be happening 10 recursive calls deep though, should it? (This error is occurring several calls deep into the call stack on this function alone. No threading)
    – pbj
    Mar 1, 2011 at 1:00
  • 3
    It might be that the final pointer in your list is not NULL but an uninitialized (random) value. Or potentially a pointer to an object that has already been freed.
    – Alex Deem
    Mar 1, 2011 at 1:06
3

It sounds like the last node in the list has an invalid value for next - it should be NULL. Is next definitely initialised to NULL in the default constructor for this class? If it isn't then that might be the problem.

2

Visual Studio isn't throwing the runtime error, but I know what you mean.

We need a little more code than this though. Is return length a method of your class list? Where is next declared? Where is it initialized? Most likely, next is an uninitialized pointer or a pointer to deallocated memory. At the very least, it is a pointer to an invalid location, thus the access violation error.

Initialize next appropriately. Checking if a pointer is null is the only appropriate mechanism for pointer validity.

Edit: It sounds like you're unfamiliar with pointer validity in general. You can not check if a pointer points to a valid location. You can only check if a pointer points to the universally invalid location null or 0. So... if you don't have a pointer pointing to something, you should initialize or set it as null or 0, that is its purpose.

2
  • The code I provided is checking the validity of the next pointer, is it not?
    – pbj
    Mar 1, 2011 at 0:58
  • It is checking if next points to anything at all. I'll revise my answer to give a little more information. Mar 1, 2011 at 1:09
1

Considering your description, it appears that next is not null, but still invalid. E.g. if your ListNode::RemoveNextNode() function had a bug where it did delete the next node but failed to change this->next. In that case you have a pointer to deallocated memory. You can't test for that; you have to make sure it doesn't happen.

In general, of course, the solution is to use std::list.

1

In C++ variables are not guaranteed to be initialized with anything sensible. If you create a variable of: MyType* next;

The next pointer can be NULL or it can be any other value. This value will be junk, but will not be zero and will cause your access violation.

To guarantee that next is NULL you must ensure that it is set to NULL in the object's constructor.

See Variable initialization in C++ for more information.

0
template <typename T_TYPE, typename T_RET, typename T_DELETE_POLICY>
class ToRef: public boost::tuple<T_TYPE*const&, T_RET, T_DELETE_POLICY>
{
public:
    ToRef<T_TYPE, T_RET, T_DELETE_POLICY>(T_TYPE*const& p_type, const T_DELETE_POLICY delete_policy)
    : boost::tuple<T_TYPE*const&, T_RET, T_DELETE_POLICY>(p_type, p_type!=NULL, delete_policy)
{
}
};

template <typename T_TYPE>
class ToRef<T_TYPE, bool, bool>: public boost::tuple<T_TYPE*const&, bool, bool>
{
public:
    explicit ToRef<T_TYPE, bool, bool>(T_TYPE*const& p_type, const bool delete_policy)
    :boost::tuple<T_TYPE*const&, bool, bool>(p_type, p_type!=NULL, delete_policy)
{
}

~ToRef()
{
    //delete policy
    if (get<2>())
    {
        if (NULL != get<0>())
            delete get<0>();
        const_cast<T_TYPE*&>(get<0>())= NULL;
    }
}

private:
    ToRef<T_TYPE, bool, bool>(ToRef<T_TYPE, bool, bool>& copy){};
    ToRef<T_TYPE, bool, bool>& operator = (ToRef<T_TYPE, bool, bool>& rhs){};

public:
    bool is_valid(void) const
    {
        //validity of the pointer.
        return get<1>();
    }

    T_TYPE& r_get(void) const
    {
        if (is_valid())
            return *get<0>();
        throw std::string("Invalid Pointer");
    }
protected:
    T_TYPE*const & p_get(void) const
    {
        return get<0>();
    }
};

//use it to safely access the pointers.
//if block is an overhead here.
#define safe_access_start(Ref) if (Ref.is_valid()) {
#define safe_access_end } 

//faster mode but unsafe. exception handling takes care of preventing 
//unhandled exception to cascade to the top.
#define fast_access_start try {
#define fast_access_end }catch (std::string s_exception){ TRACE("%s, %d, 
   %s", __FILE__, __LINE__, LPCTSTR(s_exception.c_str()));}

In your case it might work like this

 int returnLength() 
    {
        if (!next) return 1;

        sa::ToRef<Stest, bool, bool> testRef(sTest, false);

        safe_access_start(testRef)
        return 1 + testRef.r_get().returnLength();
        safe_access_end

        return 1;

    }
1
  • 1
    It is good practice to include some explanation to an answer, not just code. I suggest to add one. Jan 17, 2013 at 22:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.