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

[swss-common] Remove object type argument from json array() constructor call #466

Closed
wants to merge 2 commits into from

Conversation

rajkumar38
Copy link
Contributor

Default value set to value_t::array in constructor.
Issue: sonic-net/sonic-sairedis#801

Signed-off-by: Rajkumar Pennadam Ramamoorthy rpennadamram@marvell.com

…or call.

Default value set to value_t::array in constructor.
Issue: sonic-net/sonic-sairedis#801

Signed-off-by: Rajkumar Pennadam Ramamoorthy <rpennadamram@marvell.com>
@rajkumar38
Copy link
Contributor Author

@kcudnik . Please review.

return basic_json(init, false, value_t::array);
return basic_json(init, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is interesting, since array type was explicitly passed as argument, can you also add unittest for that scenario that was causing crash ? in this repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an UT case in file, json_ut.cpp.

…or call.

Default value set to value_t::array in constructor.
Issue: sonic-net/sonic-sairedis#801

Signed-off-by: Rajkumar Pennadam Ramamoorthy <rpennadamram@marvell.com>
@rajkumar38
Copy link
Contributor Author

Pls hold on this PR.
Even with this work-around, we are seeing same crash after several hours.

@kcudnik
Copy link
Contributor

kcudnik commented Mar 12, 2021

Pls hold on this PR.
Even with this work-around, we are seeing same crash after several hours.

please diagnose and add those crashes to unittests here also when fixed

@kcudnik
Copy link
Contributor

kcudnik commented Apr 3, 2021

this one also after verification, could be closed, since issue is most likely related to cross compiler issues on armhf

@kcudnik
Copy link
Contributor

kcudnik commented Jul 5, 2021

closing, as this is no longer needed since issue 801 was resolved

@kcudnik kcudnik closed this Jul 5, 2021
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.

2 participants