Skip to content

Conversation

terratec
Copy link
Contributor

For issue #1242

Copy link

github-actions bot commented Sep 22, 2025

Test Results

20 tests  ±0   20 ✅ ±0   0s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 032a3c9. ± Comparison against base commit ac7e70f.

♻️ This comment has been updated with latest results.

}
statisticsDataEnd = newData; // set DataEnd to new data

StatisticsNextNodePtr next = statisticsDataEnd->next;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be a linked list now, it can just be an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we instead use a virtual index (0-720) for data retrieval and convert it to the current index (e.g. 359-719-0-358)?
Or should we return all the copied and reordered data at once with newly allocated memory?

Copy link
Collaborator

@mutatrum mutatrum Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if I understand the question. I would keep stack of the size and the start of the data in the array (head), then you can convert any position to the array position by modulo 720.

if (NULL != statisticsDataStart) {
removeStatisticsBuffer();

StatisticsNodePtr buffer = (StatisticsNodePtr)malloc(sizeof(struct StatisticsData) * maxDataCount);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use heap_caps_malloc with MALLOC_CAP_DEFAULT | MALLOC_CAP_SPIRAM to force it into PSRAM?

@mutatrum mutatrum linked an issue Sep 23, 2025 that may be closed by this pull request
@WantClue WantClue added this to the 2.11.0 milestone Sep 25, 2025
@mutatrum mutatrum added the bug Something isn't working label Sep 26, 2025
@WantClue
Copy link
Collaborator

WantClue commented Oct 6, 2025

There is a current potential for a memory leak with duplicate staticbuffer creating.

my suggested change would be

if (NULL == statisticsBuffer) {
-        createStatisticsBuffer();
+        pthread_mutex_lock(&statisticsDataLock);
+        if (NULL == statisticsBuffer) {
+            pthread_mutex_unlock(&statisticsDataLock);
+            createStatisticsBuffer();
+        } else {
+            pthread_mutex_unlock(&statisticsDataLock);
+        }
     }

Also please make sure to simplify it and I agree with @mutatrum that there is no need for a linked list

@WantClue WantClue added the help wanted Extra attention is needed label Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Static allocation of statistics data

3 participants