Use placement new for ETSTimer - no heap fragmentation (#6164)

* Use placement new for ETSTimer - no heap fragmentation, new/delete semantics unchanged.

* Make change completely invisible to derived classes at compile-time.

* Fix "sizeof() incomplete type ETSTimer" error.

* C++ reinterpret_cast<> instead of C-style cast.

void* instead of uint32_t - fixes x86_64 server compiles.

* Simplify casts.

* Revert to complete placement new treatment of ETSTimer member.

* Cleanup includes

* Fix omitted casts

* Change per review #6164 (review)

* wtf - local compile didn't catch this sloppy mistake

* Resolves review #6164 (comment)

* Reviewer stated that floating point operations are inlined, software operations -

reduce number of code spots to one.

libraries/Ticker/Ticker.cpp

@@ -1,9 +1,9 @@
-/* 
+/*
   Ticker.cpp - esp8266 library that calls functions periodically
 
   Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
   This file is part of the esp8266 core for Arduino environment.
- 
+
   This library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
@@ -19,21 +19,20 @@
   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 */
 
-#include <stddef.h>
-#include <stdint.h>
-
 #include "c_types.h"
 #include "eagle_soc.h"
-#include "ets_sys.h"
 #include "osapi.h"
 
-static const int ONCE   = 0;
-static const int REPEAT = 1;
-
 #include "Ticker.h"
 
+namespace
+{
+	constexpr int ONCE = 0;
+	constexpr int REPEAT = 1;
+}
+
 Ticker::Ticker()
-: _timer(nullptr)
+	: _timer(nullptr)
 {
 }
 
@@ -42,19 +41,24 @@ Ticker::~Ticker()
 	detach();
 }
 
-void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg)
+void Ticker::_attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg)
+{
+	_attach_ms(1000 * seconds, repeat, callback, arg);
+}
+
+void Ticker::_attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg)
 {
 	if (_timer)
 	{
 		os_timer_disarm(_timer);
 	}
 	else
 	{
-		_timer = new ETSTimer;
+		_timer = &_etsTimer;
 	}
 
-	os_timer_setfn(_timer, reinterpret_cast<ETSTimerFunc*>(callback), reinterpret_cast<void*>(arg));
-	os_timer_arm(_timer, milliseconds, (repeat)?REPEAT:ONCE);
+	os_timer_setfn(_timer, callback, arg);
+	os_timer_arm(_timer, milliseconds, (repeat) ? REPEAT : ONCE);
 }
 
 void Ticker::detach()
@@ -63,25 +67,18 @@ void Ticker::detach()
 		return;
 
 	os_timer_disarm(_timer);
-	delete _timer;
 	_timer = nullptr;
 	_callback_function = nullptr;
 }
 
 bool Ticker::active() const
 {
-	return (bool)_timer;
+	return _timer;
 }
 
 void Ticker::_static_callback(void* arg)
 {
-	Ticker* _this = (Ticker*)arg;
-	if (_this == nullptr)
-	{
-		return;
-	}
-	if (_this->_callback_function)
-	{
+	Ticker* _this = reinterpret_cast<Ticker*>(arg);
+	if (_this && _this->_callback_function)
 		_this->_callback_function();
-	}
 }

libraries/Ticker/Ticker.h

@@ -1,9 +1,9 @@
-/* 
+/*
   Ticker.h - esp8266 library that calls functions periodically
 
   Copyright (c) 2014 Ivan Grokhotkov. All rights reserved.
   This file is part of the esp8266 core for Arduino environment.
- 
+
   This library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Lesser General Public
   License as published by the Free Software Foundation; either
@@ -22,22 +22,16 @@
 #ifndef TICKER_H
 #define TICKER_H
 
-#include <stdint.h>
-#include <stdbool.h>
-#include <stddef.h>
 #include <functional>
 #include <Schedule.h>
-
-extern "C" {
-	typedef struct _ETSTIMER_ ETSTimer;
-}
+#include <ets_sys.h>
 
 class Ticker
 {
 public:
 	Ticker();
 	~Ticker();
-	typedef void (*callback_t)(void);
+
 	typedef void (*callback_with_arg_t)(void*);
 	typedef std::function<void(void)> callback_function_t;
 
@@ -48,8 +42,8 @@ class Ticker
 
 	void attach(float seconds, callback_function_t callback)
 	{
-		_callback_function = callback;
-		attach(seconds, _static_callback, (void*)this);
+		_callback_function = std::move(callback);
+		_attach_s(seconds, true, _static_callback, this);
 	}
 
 	void attach_ms_scheduled(uint32_t milliseconds, callback_function_t callback)
@@ -59,27 +53,25 @@ class Ticker
 
 	void attach_ms(uint32_t milliseconds, callback_function_t callback)
 	{
-		_callback_function = callback;
-		attach_ms(milliseconds, _static_callback, (void*)this);
+		_callback_function = std::move(callback);
+		_attach_ms(milliseconds, true, _static_callback, this);
 	}
 
 	template<typename TArg>
 	void attach(float seconds, void (*callback)(TArg), TArg arg)
 	{
-		static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes");
+		static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
 		// C-cast serves two purposes:
 		// static_cast for smaller integer types,
 		// reinterpret_cast + const_cast for pointer types
-		uint32_t arg32 = (uint32_t)arg;
-		_attach_ms(seconds * 1000, true, reinterpret_cast<callback_with_arg_t>(callback), arg32);
+		_attach_s(seconds, true, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
 	}
 
 	template<typename TArg>
 	void attach_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
 	{
-		static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes");
-		uint32_t arg32 = (uint32_t)arg;
-		_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), arg32);
+		static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
+		_attach_ms(milliseconds, true, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
 	}
 
 	void once_scheduled(float seconds, callback_function_t callback)
@@ -89,8 +81,8 @@ class Ticker
 
 	void once(float seconds, callback_function_t callback)
 	{
-		_callback_function = callback;
-		once(seconds, _static_callback, (void*)this);
+		_callback_function = std::move(callback);
+		_attach_s(seconds, false, _static_callback, this);
 	}
 
 	void once_ms_scheduled(uint32_t milliseconds, callback_function_t callback)
@@ -100,36 +92,38 @@ class Ticker
 
 	void once_ms(uint32_t milliseconds, callback_function_t callback)
 	{
-		_callback_function = callback;
-		once_ms(milliseconds, _static_callback, (void*)this);
+		_callback_function = std::move(callback);
+		_attach_ms(milliseconds, false, _static_callback, this);
 	}
 
 	template<typename TArg>
 	void once(float seconds, void (*callback)(TArg), TArg arg)
 	{
-		static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach() callback argument size must be <= 4 bytes");
-		uint32_t arg32 = (uint32_t)(arg);
-		_attach_ms(seconds * 1000, false, reinterpret_cast<callback_with_arg_t>(callback), arg32);
+		static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
+		_attach_s(seconds, false, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
 	}
 
 	template<typename TArg>
 	void once_ms(uint32_t milliseconds, void (*callback)(TArg), TArg arg)
 	{
-		static_assert(sizeof(TArg) <= sizeof(uint32_t), "attach_ms() callback argument size must be <= 4 bytes");
-		uint32_t arg32 = (uint32_t)(arg);
-		_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), arg32);
+		static_assert(sizeof(TArg) <= sizeof(void*), "attach() callback argument size must be <= sizeof(void*)");
+		_attach_ms(milliseconds, false, reinterpret_cast<callback_with_arg_t>(callback), (void*)arg);
 	}
 
 	void detach();
 	bool active() const;
 
-protected:	
-	void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, uint32_t arg);
-	static void _static_callback (void* arg);
-
 protected:
+	void _attach_ms(uint32_t milliseconds, bool repeat, callback_with_arg_t callback, void* arg);
+	static void _static_callback(void* arg);
+
 	ETSTimer* _timer;
 	callback_function_t _callback_function = nullptr;
+
+private:
+	void _attach_s(float seconds, bool repeat, callback_with_arg_t callback, void* arg);
+	//char _etsTimerMem[sizeof(ETSTimer)];
+	ETSTimer _etsTimer;
 };