Skip to content

fightwarn - avoid variable-length arrays, their support became optional after C99#910

Merged
jimklimov merged 8 commits intonetworkupstools:masterfrom
jimklimov:fightwarn-vla
Nov 29, 2020
Merged

fightwarn - avoid variable-length arrays, their support became optional after C99#910
jimklimov merged 8 commits intonetworkupstools:masterfrom
jimklimov:fightwarn-vla

Conversation

@jimklimov
Copy link
Copy Markdown
Member

Follows up from #823 / #844 efforts, this PR avoids initializing variable-length arrays in the code. In most cases it was changed to dynamic xmalloc/free-before-return handling. A couple of subsequent fixes for rhino.c also got here, which depend on these changes.

In the solis.c driver it was just replaced by the macro which (in header) initializes the unmodifiable (right?) typed variable:

#define PACKET_SIZE 25
static const int packet_size = PACKET_SIZE;

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Nov 26, 2020

This pull request introduces 1 alert when merging f0167e6 into 1d9d22a - view on LGTM.com

new alerts:

  • 1 for FIXME comment

@jimklimov jimklimov added need testing Code looks reasonable, but the feature would better be tested against hardware or OSes ready / code review Author (and CI) consider the PR worthy of human rewievers' time labels Nov 27, 2020
@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Nov 28, 2020

This pull request introduces 1 alert when merging 836a4fb into e3a5544 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

Copy link
Copy Markdown
Member

@clepple clepple left a comment

Choose a reason for hiding this comment

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

Changes seem reasonable, but as with a lot of the other changes, we'll want to mention in the release notes (or better yet, on the lists beforehand) that testing is needed.

@jimklimov jimklimov merged commit 6382ccc into networkupstools:master Nov 29, 2020
@jimklimov jimklimov deleted the fightwarn-vla branch November 29, 2020 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

need testing Code looks reasonable, but the feature would better be tested against hardware or OSes ready / code review Author (and CI) consider the PR worthy of human rewievers' time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants