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:avoid division by zero foir empty segment #2301

Merged
merged 3 commits into from
May 3, 2021

Conversation

GabrieleManduchi
Copy link
Contributor

No description provided.

@GabrieleManduchi
Copy link
Contributor Author

retest this please

Copy link
Contributor

@zack-vii zack-vii left a comment

Choose a reason for hiding this comment

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

Hi Gabrielle, can you please use clang-format to get the format checked out.
You can do this by running the ./clang-format,sh script which does the required fixups as well.

./clang-format.sh xtreeshr

@GabrieleManduchi
Copy link
Contributor Author

Hi Timo, after installing clang, I get a lot of errors about unknown key 'ReflowComments'

@GabrieleManduchi
Copy link
Contributor Author

it is better you do it on a system where clang works properly

@zack-vii
Copy link
Contributor

i did but should the line not be

int itemSize = (numData == 0) ? 0 : dataD->arsize / numData;

@zack-vii
Copy link
Contributor

Furthermore you should add a test with empty segments to check it this is a clean solution (memory safe).
numData == 0 is if numDims <= 1 : line 234

  int numTimebase, numData = (numDims <= 1)
                                 ? (int)(dataD->arsize / dataD->length)
                                 : dims[numDims - 1];

Looks like it 'numData == 0' -> numTimebase = 0: line 246

  // Check data array too short
  if (numData < numTimebase)
    numTimebase = numData;

which will lead to a memory violation around line 267

  if (endD)
  {
    status = XTreeConvertToDouble(endD, &end);
    if (STATUS_NOT_OK)
      return status;
    if (end > fulltimebase[numTimebase - 1])
      end = fulltimebase[numTimebase - 1];
  }
  else
    end = fulltimebase[numTimebase - 1];

@zack-vii
Copy link
Contributor

to me it looks like the easiest way to avoid downstream issues is to short circuit the arsize == 0 case.
something like

if (arsize == 0)
{
copy in to out;
}
else
{
do normal resampling
}
form answer;

@GabrieleManduchi
Copy link
Contributor Author

right!
I have changed things to handle empty segments explicitly.
Moreover I discovered another error: at line 241, the input signal was wrongly passed as pointer to pointer

Copy link
Contributor

@AndreaRigoni AndreaRigoni left a comment

Choose a reason for hiding this comment

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

Ok !

@GabrieleManduchi GabrieleManduchi merged commit 6c4adc4 into alpha May 3, 2021
@GabrieleManduchi GabrieleManduchi deleted the gm-fix-resample branch May 3, 2021 09:33
MDSplusBuilder pushed a commit that referenced this pull request Jun 15, 2021
* tag 'alpha_release-7-131-1': (27 commits)
  Fix: Add extra selection for different trigger sources (#2314)
  Feature: MdsIpThreadStatic ConnectionList; should fix deadlock and be faster (#2288)
  Gm beans (#2312)
  Build: fixed alpine externals, removed python dependency if numpy is required
  Fix:jTraverer and jScope fixezs (#2310)
  Build: ub20 does not have python_numpy (#2309)
  Fix:changed event API (#2306)
  Build: debian cleanup repo
  Build: alpine fixed noarch
  Build: cleanup buildroot for debian
  Build: fixed Docker_NETWORK cleanup if not supported
  Build: network cannot be none for udp events
  Build: fixed build scripts to maintain multiarch files
  Build: isolate docker containers with --network=none
  Build: platform builds on empty/new release folder and format (#2304)
  Fix: Refactor out _MOVC3 macro (#2300)
  Tests: splits up python tests to get finer results (#2296)
  Build: fixed windows 32 to ship libgcc_s_dw2 as SEH is not supported for 32 bit yet (#2302)
  Fix:avoid division by zero foir empty segment (#2301)
  Tests: fixed fc32 helgrind suppression file
  ...
MDSplusBuilder pushed a commit that referenced this pull request Jun 15, 2021
* tag 'alpha_release-7-131-1': (27 commits)
  Fix: Add extra selection for different trigger sources (#2314)
  Feature: MdsIpThreadStatic ConnectionList; should fix deadlock and be faster (#2288)
  Gm beans (#2312)
  Build: fixed alpine externals, removed python dependency if numpy is required
  Fix:jTraverer and jScope fixezs (#2310)
  Build: ub20 does not have python_numpy (#2309)
  Fix:changed event API (#2306)
  Build: debian cleanup repo
  Build: alpine fixed noarch
  Build: cleanup buildroot for debian
  Build: fixed Docker_NETWORK cleanup if not supported
  Build: network cannot be none for udp events
  Build: fixed build scripts to maintain multiarch files
  Build: isolate docker containers with --network=none
  Build: platform builds on empty/new release folder and format (#2304)
  Fix: Refactor out _MOVC3 macro (#2300)
  Tests: splits up python tests to get finer results (#2296)
  Build: fixed windows 32 to ship libgcc_s_dw2 as SEH is not supported for 32 bit yet (#2302)
  Fix:avoid division by zero foir empty segment (#2301)
  Tests: fixed fc32 helgrind suppression file
  ...
MDSplusBuilder pushed a commit that referenced this pull request Jun 17, 2021
* tag 'alpha_release-7-131-1': (27 commits)
  Fix: Add extra selection for different trigger sources (#2314)
  Feature: MdsIpThreadStatic ConnectionList; should fix deadlock and be faster (#2288)
  Gm beans (#2312)
  Build: fixed alpine externals, removed python dependency if numpy is required
  Fix:jTraverer and jScope fixezs (#2310)
  Build: ub20 does not have python_numpy (#2309)
  Fix:changed event API (#2306)
  Build: debian cleanup repo
  Build: alpine fixed noarch
  Build: cleanup buildroot for debian
  Build: fixed Docker_NETWORK cleanup if not supported
  Build: network cannot be none for udp events
  Build: fixed build scripts to maintain multiarch files
  Build: isolate docker containers with --network=none
  Build: platform builds on empty/new release folder and format (#2304)
  Fix: Refactor out _MOVC3 macro (#2300)
  Tests: splits up python tests to get finer results (#2296)
  Build: fixed windows 32 to ship libgcc_s_dw2 as SEH is not supported for 32 bit yet (#2302)
  Fix:avoid division by zero foir empty segment (#2301)
  Tests: fixed fc32 helgrind suppression file
  ...
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.

3 participants