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

ndpireader: add json output back. #1509

Merged
merged 1 commit into from
Apr 1, 2022
Merged

ndpireader: add json output back. #1509

merged 1 commit into from
Apr 1, 2022

Conversation

vitalivanov
Copy link
Contributor

  • partial revert of:
    commit 51cfdfb
    Author: Luca Deri [email protected]
    Date: Sun Nov 17 17:51:45 2019 +0100
    Removed unused JSON-C code

  • Json option is changed from 'j' to 'k' as it's used in the new codebase.

  • use HAVE_LIBJSON_C instead of HAVE_JSON_C.

  • tabs vs spaces clean ups.

Signed-off-by: Vitaliy Ivanov [email protected]

Conflicts:
example/ndpiReader.c

@vitalivanov
ndpireader: add json output back.
4e2e5b0 
- partial revert of:
    commit 51cfdfb
    Author: Luca Deri <[email protected]>
    Date: Sun Nov 17 17:51:45 2019 +0100
      Removed unused JSON-C code

- Json option is changed from 'j' to 'k' as it's used in the new codebase.
- use HAVE_LIBJSON_C instead of HAVE_JSON_C.
- tabs vs spaces clean ups.

Signed-off-by: Vitaliy Ivanov <[email protected]>

 Conflicts:
	example/ndpiReader.c
@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vitalivanov vitalivanov mentioned this pull request Mar 31, 2022
Closed
@lgtm-com
Copy link

lgtm-com bot commented Mar 31, 2022

This pull request introduces 5 alerts when merging 4e2e5b0 into 6677d69 - view on LGTM.com

new alerts:

  • 4 for Potentially overflowing call to snprintf
  • 1 for Local variable hides global variable

@lucaderi lucaderi merged commit 61bc981 into ntop:dev Apr 1, 2022
@lucaderi
Copy link
Member

lucaderi commented Apr 1, 2022

Thanks you

@IvanNardi
Copy link
Collaborator

This patch broke the ndpiReader output when "-v2" option is used.
That means that unit tests are now completely broken.
There are also some (trivial) compilation errors that need to be fixed.
@vitalivanov , could you take a look, please?

@lucaderi
Copy link
Member

lucaderi commented Apr 1, 2022

I am also looking at it as I have realized it too

lucaderi added a commit that referenced this pull request Apr 1, 2022
@lucaderi
Fixed incompatibilities due to #1509
32a7d42
@lucaderi
Copy link
Member

lucaderi commented Apr 1, 2022

@vitalivanov I have fixed the dev branch, please send a PR for the stable branch

@vitalivanov
Copy link
Contributor Author

please send a PR for the stable branch
ok i will

@lucaderi
@IvanNardi

a.
4 for Potentially overflowing call to snprintf
1 for Local variable hides global variable

these are part of old npireader json patch...

b.
tabs vs spaces - yes, agree, but reading that was a pain...

c. v2 - if luca addressed this in dev - I would update to 4.2-stable.
it still works with v4:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vitalivanov @lucaderi @IvanNardi