Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor errors handling and memory management #17

Open
dodikk opened this issue Feb 12, 2014 · 7 comments
Open

Poor errors handling and memory management #17

dodikk opened this issue Feb 12, 2014 · 7 comments

Comments

@dodikk
Copy link
Contributor

dodikk commented Feb 12, 2014

For example :

- (NSSet *)protocols {
    if (!_protocols) {
        Protocol *__unsafe_unretained *protocols;
        uint protocolsCount = 0;
        protocols = class_copyProtocolList(_objcClass, &protocolsCount);
        NSMutableSet *mutableProtocols = [NSMutableSet setWithCapacity:protocolsCount];
        for (uint protocolIndex = 0; protocolIndex != protocolsCount; ++protocolIndex) {
            Protocol *protocol = protocols[protocolIndex];
            [mutableProtocols addObject:protocol];
        }

// What makes you sure "protocols" variable is not NULL ?
free(protocols);

        _protocols = [NSSet setWithSet:mutableProtocols];
    }
    return _protocols;
}
@dodikk
Copy link
Contributor Author

dodikk commented Feb 12, 2014

- (void)collectClasses
{
#ifdef __IPHONE_OS_VERSION_MIN_REQUIRED
    uint classesCount;
    const char *imageName = class_getImageName(object_getClass(self));
    const char **classNames = objc_copyClassNamesForImage(imageName, &classesCount);
    for (uint index = 0; index < classesCount; index++) {
        Class nextClass = objc_getClass(classNames[index]);

// what if push_back() throws an exception?
_cachedClasses.push_back(nextClass);

// you'll never reach ::free() statement

    }
    free(classNames);

@AlexDenisov
Copy link
Contributor

Premature optimization?
What should I do in case when [NSArray new] returns nil?

@AlexDenisov
Copy link
Contributor

I understand what you mean, but, IMHO, there a lot of cases when something might be broken, should we cover all them?

Anyway, if you see a big problem with this part of code - feel free to send a PR.

@dodikk
Copy link
Contributor Author

dodikk commented Feb 12, 2014

It's not about optimizations. It's rather about proper resources management (memory) and avoiding crashes.

What should I do in case when [NSArray new] returns nil?

Nothing since it is safe to send messages to a nil object
Still, the case with invoking ::free( NULL ) is completely different since it will eventually lead to a crash.

@dodikk
Copy link
Contributor Author

dodikk commented Feb 12, 2014

there a lot of cases when something might be broken, should we cover all them?

Critical crashes should be addressed in no doubt.
Still, you are the maintainer and it's up to you setting the priorities.

@AlexDenisov
Copy link
Contributor

Thank you, I've got your point and will check and fix such issues.

@anton-matosov
Copy link

7.20.3.2 The free function

Synopsis

#include <stdlib.h>
void free(void *ptr);
Description

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs.

See ISO-IEC 9899.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants