Skip to content

Refactor: Generalize Code by Removing Vendor and Android-Specific Elements #154

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

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

quic-vkatoch
Copy link
Contributor

Refactored the codebase to remove vendor-specific and Android-specific elements, making the project more general and platform-independent.

@@ -3548,12 +3483,6 @@ static int open_shell(int domain_id, apps_std_FILE *fh, int unsigned_shell) {
if (domain == SDSP_DOMAIN_ID && fh != NULL) {
nErr = AEE_SUCCESS;
*fh = -1;
} else {
FARF(ERROR,
Copy link
Contributor

Choose a reason for hiding this comment

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

should not completely remove the error log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error log contains vendor-specific location macros.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, the error log needs to be fixed rather than being completely removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll only be logging the paths that have been actually checked.

#endif

#include <string.h>
#include <errno.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

can you highlight what is changed in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing has changed in this file.

"vendor.fastrpc.debug.systrace",
"vendor.fastrpc.debug.pddump",
"persist.vendor.fastrpc.process.attrs",
"ro.build.type"};
const char *ANDROID_DEBUG_VAR_NAME[] = {"fastrpc.process.attrs",
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 also Android-specific var and was missed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. I am working on the cleanup and will include it in the next patch.

@@ -241,26 +230,6 @@ const char *ENV_DEBUG_VAR_NAME[] = {"FASTRPC_PROCESS_ATTRS",
"FASTRPC_DEBUG_PDDUMP",
"FASTRPC_PROCESS_ATTRS_PERSISTENT",
"ro.debuggable"};
Copy link
Contributor

Choose a reason for hiding this comment

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

ro.debuggable is also an Android definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check and remove it.

@quic-ekangupt
Copy link
Contributor

Please check if you can break the change into multiple commits instead of a single commit.

Vinayak Katoch added 5 commits April 25, 2025 12:01
definitions in apps_std_internal.h

Signed-off-by: Vinayak Katoch <[email protected]>
conditional include for systrace in fastrpc_trace.h

Signed-off-by: Vinayak Katoch <[email protected]>
listener.c, and update function names in listener.c

Signed-off-by: Vinayak Katoch <[email protected]>
update Makefile.am, and update PROPERTY_VALUE_MAX definition.

Signed-off-by: Vinayak Katoch <[email protected]>
fastrpc_apps_user.c

Signed-off-by: Vinayak Katoch <[email protected]>
for all build types.

Signed-off-by: Vinayak Katoch <[email protected]>
#endif
}
if (persist_buf.buf == NULL && debug_build_type) {
if (persist_buf.buf == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These are separate changes. Why are you squashing them into a single commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have separated the changes into multiple commits and it's no longer a single commit. I'm checking if it could be separated any further.

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