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.

Sign up for GitHub

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

Jump to bottom

utl/spdemo: set default value for string-based parameter. #116

Merged
merged 1 commit into from
Nov 28, 2018
Merged

utl/spdemo: set default value for string-based parameter. #116

merged 1 commit into from
Nov 28, 2018

Conversation

dennisguse
Copy link
Member

ATTENTION: This solution is just a guess and needs testing (especially on MacOS).

This might be a fix for #46.

Potential explanation:
I believe the issue results from using strcpy on src == dst.
This is not allowed as src and dst should not overlap.

@dennisguse dennisguse self-assigned this Nov 13, 2018
ludomal
ludomal requested changes Nov 14, 2018
View reviewed changes
Copy link
Member

@ludomal ludomal left a comment

Choose a reason for hiding this comment

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

This would work for spdemo, but shall we find FIND_PAR_S macro instead?

It may be better to fix the macro so that we will not stumble on this issue again. A condition that checks that prevents the strcpy if the pointers are the same should work fine:

#define FIND_PAR_S(p,msg,i,dft) \
{  if (i != dft) { strcpy(i,(argc>p)?argv[p]:dft);}\
   fprintf (stderr,"%s%s\n",msg,i); }

@dennisguse
Copy link
Member Author

dennisguse commented Nov 14, 2018 •
edited
Loading

Nice that it fixes the issue. About the macros: I prefer replacing them with small functions. Easier to maintain. (And also fixes the issue).
See #45 and #46.

@ludomal
Copy link
Member

ludomal commented Nov 14, 2018

I would be in favor of removing the macros but it looks like it was necessary for some of the old compilers. We need to find out which platform was problematic and whether we still want to support them.

Shall we keep this task for STL2019? These macros/functions would probably not be needed anymore the day we look into #8.

@dennisguse
Copy link
Member Author

I will fix the macro tomorrow. You are right removing the macros should be done after STL2018.

@dennisguse
Copy link
Member Author

dennisguse commented Nov 15, 2018 •
edited
Loading

Fixed macro as well.
Now uses memmove instead of strcpy, as memmove copes with overlapping memory.

@ludomal Could you test it and merge if ready?

@dennisguse dennisguse added the bug label Nov 15, 2018
@dennisguse dennisguse added this to the STL2018 milestone Nov 15, 2018
@dennisguse
Copy link
Member Author

In addition: #117 enables continuous integration for MacOS.
I would prefer merging #117 before #116.

@dennisguse
Copy link
Member Author

Rebased to include Travis4MacOS.
On MacOS spdemo does not abort with Trap 6.

ludomal
ludomal requested changes Nov 16, 2018
View reviewed changes
Copy link
Member

@ludomal ludomal left a comment •
edited
Loading

Choose a reason for hiding this comment

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

-- moved --

ludomal
ludomal reviewed Nov 17, 2018
View reviewed changes
src/utl/ugstdemo.h
@@ -183,7 +183,7 @@
fprintf (stderr,"%s%c\n",msg,C);}

#define FIND_PAR_S(p,msg,i,dft) \
{ strcpy(i,(argc>p)?argv[p]:dft);\
{ memmove(i,(argc>p)?argv[p]:dft, strlen((argc>p)?argv[p]:dft));\
Copy link
Member

Choose a reason for hiding this comment

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

That works.

@ludomal ludomal mentioned this pull request Nov 17, 2018
Open
@dennisguse dennisguse force-pushed the spdemo-fix branch 2 times, most recently from 3f1f401 to ab538b5 Compare November 18, 2018 21:15
@dennisguse
Copy link
Member Author

Now only fixing FIND_PAR_S.

ludomal
ludomal approved these changes Nov 28, 2018
View reviewed changes
Copy link
Member

@ludomal ludomal left a comment

Choose a reason for hiding this comment

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

Rebased to only include changes to src/utl/ugstdemo.h

src/utl/spdemo.c Outdated
@@ -535,7 +535,7 @@ int main (int argc, char *argv[]) {
FIND_PAR_L (6, "_No. of Blocks: .............. ", N2, N2);
FIND_PAR_L (7, "_Resolution of input data: ... ", resolution, resolution);
FIND_PAR_I (8, "_Sync header? (1=Yes,0=No) ... ", sync, sync);
FIND_PAR_S (9, "_Left of right adjusted? ..... ", just, just);
FIND_PAR_S (9, "_Left of right adjusted? ..... ", just, "right");
Copy link
Member

Choose a reason for hiding this comment

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

just has a default value set at declaration (right).

This change should be reverted as it overwrites the justification set with -left switch.

src/utl/scaldemo.c Outdated
@@ -287,6 +287,7 @@ int main (int argc, char *argv[]) {
/* Read parameters for processing */
GET_PAR_S (1, "_Input File: ........................... ", FileIn);
GET_PAR_S (2, "_Output File: .......................... ", FileOut);

Copy link
Member

Choose a reason for hiding this comment

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

Are these changes necessary?

@ludomal ludomal merged commit e33d62e into dev Nov 28, 2018
@ludomal ludomal mentioned this pull request Nov 28, 2018
Closed
@dennisguse dennisguse deleted the spdemo-fix branch December 5, 2018 20:05
@MarkusMultrus MarkusMultrus mentioned this pull request Apr 15, 2023
Closed
@ludomal ludomal mentioned this pull request Apr 30, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ludomal ludomal ludomal approved these changes

Assignees

@dennisguse dennisguse

Labels
bug
Projects
None yet
Milestone
STL2018
Development

Successfully merging this pull request may close these issues.
2 participants
@dennisguse @ludomal