Make macro defined event base variables immutable (IDFGH-9106)

[ljden]

I believe the following code should not compile:

ESP_EVENT_DEFINE_BASE(MY_EVENT);
MY_EVENT="oops";

however it does as esp_event_base_t is a mutable pointer.

esp-idf/components/esp_event/include/esp_event_base.h

Lines 23 to 27 in 4c98bee

	#define ESP_EVENT_DECLARE_BASE(id) extern esp_event_base_t id
	#define ESP_EVENT_DEFINE_BASE(id) esp_event_base_t id = #id
	// Event loop library types
	typedef const char* esp_event_base_t; /**< unique pointer to a subsystem that exposes events */

I suggest that as esp_event_base_t type may be used in user code relying on the mutability of the type, the ESP_EVENT_*_BASE macros should be updated to make the variable immutable:

// Defines for declaring and defining event base
#define ESP_EVENT_DECLARE_BASE(id) extern esp_event_base_t const id
#define ESP_EVENT_DEFINE_BASE(id) esp_event_base_t const id = #id

This results in the original code no longer compiling:

error: assignment of read-only variable 'MY_EVENT'
     MY_EVENT = "oops";
     ~~~~~~~~~^~~~~~

If there is a reason for the mutability that I've missed please let me know!

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Alvin1Zhang]

Thanks for your contribution.

[0xjakob]

@ljden Thanks, I think it's a good change!

Unfortunately, the pre-commit hook fails because that file has an old license/copyright header which is rejected once it is changed. There are several solutions to this:

• You could install the pre-commit hook and run it like this: pre-commit run --from-ref=<commit-before-your-change> --to-ref=HEAD. You can later remove or deactivate your pre-commit hook if it bothers you.
• You could wait until we changed all the header files of esp_event from the IDF side. This might take several days to a week due to internal processes. Then you could rebase and try again.
• You keep it as it is but we need to amend your commit. In this case, github will see a different commit hash once it appears on master. The MR will have the status "closed" instead of "merged". Besides, your authorship of the commit will still be retained.

[ljden]

@0xjakob is updating the licence also a solution? Also, what does installing the pre-commit hook do? Will it actually change anything or would it just notify my locally that I have an issue

[0xjakob]

sha=b4d904f8f05a69f96587f7e0d1807363d985e131

[0xjakob]

@ljden Looks great! Thanks for the update.

Just for the record:

what does installing the pre-commit hook do

It installs a bunch of checkers that automatically run whenever you commit to the IDF repository. These checks are also run in our internal pipelines, hence you can't avoid fixing them and it's better to fix them before even pushing the code (ideally).

is updating the licence also a solution

The pre-commit hook will try to automatically fix license headers once you run it.

[ljden]

Makes sense, cheers for the details 👍

[0xjakob]

@ljden Your code is available on master already (i.e., it has been merged). Consequently, we're closing this PR.