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

fix flashinit panic not printing #8762

Merged
merged 11 commits into from
Dec 22, 2022
Merged

Conversation

mhightower83
Copy link
Contributor

@mhightower83 mhightower83 commented Dec 16, 2022

@mhightower83 mhightower83 changed the title fix panic not printing fix flashinit panic not printing Dec 16, 2022
cores/esp8266/flash_hal.h Outdated Show resolved Hide resolved
@d-a-v d-a-v added this to the 3.1 milestone Dec 16, 2022
@mcspr
Copy link
Collaborator

mcspr commented Dec 19, 2022

Just as a thought, don't we want to return something like this?

FlashInitResult flashinit();
struct FlashInitResult {
    bool ok { false };
    const char* err_msg;
};

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 19, 2022

flashinit() was not designed to fail, so it is 'panic'ing instead

edit: my bad, I was some merged PRs late

@mhightower83
Copy link
Contributor Author

mhightower83 commented Dec 19, 2022

Too many merge conflicts were corrected with web edit, I need to correct some stuff and do a few real tests.

@mcspr
Copy link
Collaborator

mcspr commented Dec 19, 2022

flashinit() was not designed to fail, so it is 'panic'ing instead

I just mean the return value. Struct member name tells us what we mean right there, no need to remember triple meaning of a char pointer

@mhightower83
Copy link
Contributor Author

FlashInitResult flashinit();

FWIF: I tried this method and it increases "IROM code in flash" by 16 bytes.

@mcspr mcspr merged commit 137d421 into esp8266:master Dec 22, 2022
@mcspr
Copy link
Collaborator

mcspr commented Dec 22, 2022

FlashInitResult flashinit();

FWIF: I tried this method and it increases "IROM code in flash" by 16 bytes.

Suppose, bool could go, but methods could do the thing instead

#include <cstdio>

void __panic_func(const char* file, int line, const char* func) {
    printf("panic! at %s:%d::%s\n", file, line, func);
}

struct SimpleResult {
    SimpleResult() = default;
    constexpr SimpleResult(const char* error) :
        _error(error)
    {}

    bool ok() const {
        return _error == nullptr;
    }

    const char* error() const {
        return _error;
    }

    // or, other order, depends on whats more important to replace
    void check(const char* file = __builtin_FILE(),
            int line = __builtin_LINE(),
            const char* func = __builtin_FUNCTION())
    {
        if (!ok()) {
            __panic_func(file, line, func);
        }
    }

private:
    const char* _error { nullptr };
};

int main() {
    SimpleResult what{"oops"};
    what.check();
}

@mhightower83 mhightower83 deleted the pr-autoconfig-fix branch December 22, 2022 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants