[Android] Crash when trying to open none-exsisting asset. sf::InputFileStream

[Daid]

I think I've finally run into something while trying to port my code to Android which I really think is an SFML bug.

Getting this backtrace:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000000
Stack frame #00  pc 00008f60  /system/lib/libandroid.so (AAsset_getLength+1)
Stack frame #01  pc 0000873f  /data/app-lib/eu.daid.emptyepsilon-2/libsfml-system.so (sf::priv::ResourceStream::tell()+18): Routine sf::priv::ResourceStream::tell() at ??:?
Stack frame #02  pc 00007c63  /data/app-lib/eu.daid.emptyepsilon-2/libsfml-system.so (sf::FileInputStream::open(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)+38): Routine sf::FileInputStream::open(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at ??:?
 

https://github.com/SFML/SFML/blob/master/src/SFML/System/Android/ResourceStream.cpp#L45
Can clearly result in m_file being a nullptr when the file is not found.

And the asset manager functions don't do any pointer checks:
http://osxr.org/android/source/frameworks/base/native/android/asset_manager.cpp

https://github.com/SFML/SFML/blob/master/src/SFML/System/FileInputStream.cpp#L64
Finally, this code uses "sf::priv::ResourceStream::tell()" to check if the file has actually opened, which results in the segfault, as it tries to call AAsset_getLength with a nullptr.


Added nullptr checks in all functions in
https://github.com/SFML/SFML/blob/master/src/SFML/System/Android/ResourceStream.cpp#L45
Which solves the crash for me.

[Nexus]

Thanks for the feedback!

You say you added null pointer checks. In the case of

// "Can clearly result in m_file being a nullptr when the file is not found."
ResourceStream::ResourceStream(const std::string& filename) :
m_file (NULL)
{
    ActivityStates* states = getActivity(NULL);
    Lock(states->mutex);
    m_file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN);
}

how do you react? You can't really prevent a failure at that point.

// "Finally, this code uses "sf::priv::ResourceStream::tell()" to check if the file has actually
// opened, which results in the segfault, as it tries to call AAsset_getLength with a nullptr."
    if (m_file)
        delete m_file;
    m_file = new priv::ResourceStream(filename);
    return m_file->tell() != -1;

There's a new operator right before, so m_file can never be a null pointer in the last line.

Questionable however is the if (m_file) delete m_file; statement.

[Daid]

I added the change in the Android/ResourceStream.cpp

////////////////////////////////////////////////////////////
//
// SFML - Simple and Fast Multimedia Library
// Copyright (C) 2013 Jonathan De Wachter (dewachter.jonathan@gmail.com)
//
// This software is provided 'as-is', without any express or implied warranty.
// In no event will the authors be held liable for any damages arising from the use of this software.
//
// Permission is granted to anyone to use this software for any purpose,
// including commercial applications, and to alter it and redistribute it freely,
// subject to the following restrictions:
//
// 1. The origin of this software must not be misrepresented;
//    you must not claim that you wrote the original software.
//    If you use this software in a product, an acknowledgment
//    in the product documentation would be appreciated but is not required.
//
// 2. Altered source versions must be plainly marked as such,
//    and must not be misrepresented as being the original software.
//
// 3. This notice may not be removed or altered from any source distribution.
//
////////////////////////////////////////////////////////////


////////////////////////////////////////////////////////////
// Headers
////////////////////////////////////////////////////////////
#include <SFML/System/Android/ResourceStream.hpp>
#include <SFML/System/Android/Activity.hpp>
#include <SFML/System/Lock.hpp>


namespace sf
{
namespace priv
{

////////////////////////////////////////////////////////////
ResourceStream::ResourceStream(const std::string& filename) :
m_file (NULL)
{
    ActivityStates* states = getActivity(NULL);
    Lock(states->mutex);
    m_file = AAssetManager_open(states->activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN);
}


////////////////////////////////////////////////////////////
ResourceStream::~ResourceStream()
{
    if (m_file)
        AAsset_close(m_file);
}


////////////////////////////////////////////////////////////
Int64 ResourceStream::read(void *data, Int64 size)
{
    if (!m_file)
        return -1;
    return AAsset_read(m_file, data, size);
}


////////////////////////////////////////////////////////////
Int64 ResourceStream::seek(Int64 position)
{
    if (!m_file)
        return -1;
    return AAsset_seek(m_file, position, SEEK_SET);
}


////////////////////////////////////////////////////////////
Int64 ResourceStream::tell()
{
    if (!m_file)
        return -1;
    return getSize() - AAsset_getRemainingLength(m_file);
}


////////////////////////////////////////////////////////////
Int64 ResourceStream::getSize()
{
    if (!m_file)
        return -1;
    return AAsset_getLength(m_file);
}


} // namespace priv
} // namespace sf
 

Most likely not the best solution, but this is what I changed to prevent this code from crashing on my build. (SFML-2.3.1)