Cmake and Meson Integration #9
Conversation
Get meson to build with the rbus stuff.
Cmake integration and initial code commit
Adding cimplog fix for extender support
Check CR systemready using rbus
Move connected client notification business logic from webconfig to w…
Change-Id: I3e37c85106f4326d4f7a3757b750d2136eff1a1a
Adding utils and database functionalities
Added sky specific code for accessing device mac address
Sky Specific mac address change
Hard-Coded Secrets (5)
More info on how to fix Hard-Coded Secrets in General. Insecure Processing of Data (4)
More info on how to fix Insecure Processing of Data in C/C++. Insecure Use of Dangerous Function (4)
More info on how to fix Insecure Use of Dangerous Function in C/C++. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
| werror = true | ||
| endif | ||
|
|
||
| script = join_paths(meson.source_root(), 'workaround.sh') |
There was a problem hiding this comment.
This seems wrong. meson 0.55.0 added a patch_directory option to handle locally wrapped dependencies, so you do not need to autogenerate tarball intermediaries here.
There was a problem hiding this comment.
@eli-schwartz The yocto build we're using (and can't change for a while) is using meson 0.53. We've not been able to iron out a set of LD based issues for the cross compiler calls with meson, which is why CMake got added. I've not been able to get back to trying to find a better solution for this, but good suggestion.
@guruchandru I'd rather not add the workaround code to the meson file since that couples us with a hack that hopefully we can remove sooner than later.
There was a problem hiding this comment.
In that case, at least re: my comment below you cannot rely on the [provide] section either, so you can just add the fallback: ['foo_project', 'foo_dep'] styled references to each one, and you still do not need if/else logic. :)
There was a problem hiding this comment.
As far as these wraps go... jansson could probably go into the wrapdb, the other ones look like they may be specialized enough that that is not worth it so the alternative would be somehow getting meson.build files into those repos...
| ################################################################################ | ||
|
|
||
| if meson.version().version_compare('>=0.54.0') | ||
| libcjson_dep = dependency('libcjson', version: '>=1.7.14', fallback: ['cjson']) |
There was a problem hiding this comment.
You don't need this if/else, since most of them are doing the exact same thing.
And use of the [provide] section means that new enough versions of meson automatically fall back with zero configuration, so it's not necessary to add special handling in meson.build.
| inc = include_directories(inc_base) | ||
| inc = include_directories([inc_base]) | ||
|
|
||
| src_args = ['-lcjson', '-lpthread'] |
There was a problem hiding this comment.
The former is already handled by libcjson_dep, the latter should not be handled by manual -l flags, see https://mesonbuild.com/FAQ.html#what-is-the-correct-way-to-use-threads-such-as-pthreads
include/cpeabs/cpeabs.h
Outdated
| #include <wdmp-c.h> | ||
|
|
||
| #if ! defined(DEVICE_EXTENDER) | ||
| #include <cimplog.h> |
There was a problem hiding this comment.
This should only be internal & not part of the exported API.
include/cpeabs/cpeabs.h
Outdated
| /*----------------------------------------------------------------------------*/ | ||
| /* Macros */ | ||
| /*----------------------------------------------------------------------------*/ | ||
| #define CPEABS_FREE(__x__) if(__x__ != NULL) { free((void*)(__x__)); __x__ = NULL;} else {printf("Trying to free null pointer\n");} |
There was a problem hiding this comment.
This should probably not be exported. If the data structures must be freed using a special function, it should be a function and not a macro. Generally macros like this should be made a function.
include/cpeabs/cpeabs.h
Outdated
| /* Macros */ | ||
| /*----------------------------------------------------------------------------*/ | ||
| #define CPEABS_FREE(__x__) if(__x__ != NULL) { free((void*)(__x__)); __x__ = NULL;} else {printf("Trying to free null pointer\n");} | ||
| #define UNUSED(x) (void )(x) |
There was a problem hiding this comment.
This should not be exported but in an internal only header.
include/cpeabs/cpeabs.h
Outdated
| /*----------------------------------------------------------------------------*/ | ||
| #define CPEABS_FREE(__x__) if(__x__ != NULL) { free((void*)(__x__)); __x__ = NULL;} else {printf("Trying to free null pointer\n");} | ||
| #define UNUSED(x) (void )(x) | ||
| #define MAX_BUFF_SIZE 256 |
There was a problem hiding this comment.
This should not be exported from the library. Where ever this is defined, add a comment explaining what the buffer is & any assumptions about how you came up with 256.
include/cpeabs/cpeabs.h
Outdated
| /** | ||
| * @brief Enables or disables debug logs. | ||
| */ | ||
| #if defined DEVICE_GATEWAY && defined BUILD_YOCTO |
There was a problem hiding this comment.
This block should all be internal facing only.
| } | ||
| else | ||
| { | ||
| WebcfgError("Failed to GetValue for PartnerID\n"); |
There was a problem hiding this comment.
Will this leak the partner_id value?
src/pods/impl.c
Outdated
| WebcfgError("%s : partner_id couldnt be assigned with memory\n",__func__); | ||
| return NULL; | ||
| } | ||
| ret_val = get_id_pstore(partner_id_chk,partner_id); |
There was a problem hiding this comment.
Instead of passing in memory, why not have get_id_pstore just malloc() the memory and return it?
| } | ||
| else | ||
| { | ||
| WebcfgError("Failed to GetValue for AccountID\n"); |
| return end_time; | ||
| } | ||
|
|
||
| int Get_Webconfig_URL( char *pString) |
There was a problem hiding this comment.
It might be worth looking at normalizing the function names at some point.
| WebcfgDebug("Successfully fetched Webconfig URL : [%s]. \n", url); | ||
| ret = RETURN_OK; | ||
| } | ||
| else{ |
There was a problem hiding this comment.
Please run the code formatting across the code so this is all normalized. This will reduce maintenance efforts in the future.
There was a problem hiding this comment.
I realized I don't have the code format checking code in across the board. I'm working on this & we can do it after this PR.
| /* External Functions */ | ||
| /*----------------------------------------------------------------------------*/ | ||
|
|
||
| char * getParamValue(char *paramName); |
There was a problem hiding this comment.
Add the descriptive header for getParamValue() to the header file so that users of the header file can see it easily and without needing to dig into the c source code. Same for the functions below.
| @@ -0,0 +1,48 @@ | |||
| /* SPDX-FileCopyrightText: 2021 Comcast Cable Communications Management, LLC */ | |||
There was a problem hiding this comment.
The cpeabs.h file should really hide this detail from being exported. These functions should be normalized and added there vs. having a cpeabs_rdkb.h and a cpeabs_pod.h and other files that are used by other libraries. This can live in the src directory if it is only needed internally.
Adding mods according to review
8 participants
No description provided.