Skip to content

Allow externalisation of PrintAndLog#506

Merged
pwpiwi merged 1 commit intoProxmark:masterfrom
micolous:external-printandlog
Dec 28, 2017
Merged

Allow externalisation of PrintAndLog#506
pwpiwi merged 1 commit intoProxmark:masterfrom
micolous:external-printandlog

Conversation

@micolous
Copy link
Contributor

@micolous micolous commented Dec 3, 2017

On Android, we handle stdout and logging a little differently. stdout is also sent to /dev/null. However, there are a number of global variables declared in ui.c which are useful for the rest of the application.

This adds a define EXTERNAL_PRINTANDLOG which will disable the stock PrintAndLog implementation in proxmark3. If defined, this would require that an implementer bring their own PrintAndLog function that implements the same API.

Example implementation for Android given below, which will push logs into the INFO log for the application (accessible via Logcat). A better implementation of this would pull it into the rest of the application's UI.

#include <android/log.h>
#define APPNAME "natives::PrintAndLog"

void PrintAndLog(char *fmt, ...)
{
    va_list argptr;
    va_start(argptr, fmt);
    __android_log_vprint(ANDROID_LOG_INFO, APPNAME, fmt, argptr);
}

@iceman1001
Copy link
Member

This one kind of crosses over the define of ON_DEVICE which also introduces a substitute for printandlog function. I agree with a unified way but this needs remake.

#ifndef ON_DEVICE

@micolous
Copy link
Contributor Author

micolous commented Dec 9, 2017

OK. Whatever mechanism is ideal, I'd also like something similar for printf. There are still many things which call printf and it's tedious to hook that.

How I'm using this on Android in a "real context" is that I have my version of PrintAndLog:

https://github.com/AndProx/AndProx/blob/2e25965fa0b9c4620f118e1db13ed427ec61097f/natives/src/main/cpp/fakemain.c#L51-L67

And then that gets brought into an event handler in Java:

https://github.com/AndProx/AndProx/blob/2e25965fa0b9c4620f118e1db13ed427ec61097f/natives/src/main/java/au/id/micolous/andprox/natives/Natives.java#L41-L72

And then my Activity hooks this event handler:

https://github.com/AndProx/AndProx/blob/2e25965fa0b9c4620f118e1db13ed427ec61097f/app/src/main/java/au/id/micolous/andprox/activities/CliActivity.java#L62-L64

@micolous
Copy link
Contributor Author

micolous commented Dec 26, 2017

@iceman1001 From #501:

The #506 still suffers from not unifying the different methods to abstract the printandlog function, as I commented there awhile ago.

And regarding last comment:

This one kind of crosses over the define of ON_DEVICE which also introduces a substitute for printandlog function. I agree with a unified way but this needs remake.

The referenced commit actually redirects stuff going in to the PrintAndLog function, and then that gets stubbed out "on device". Unfortunately this isn't really appropriate for my purposes as I actually want to send it somewhere else.

What you have in ui.c at the moment is two pieces of functionality:

  • logging
  • a bunch of globals get defined

The "easy way" to sort this is for the logging related functions to go live in their own file (eg: logging.c / .h), then I just provide my own version of the logging API calls (like I have done for uart.h) and not build logging.c.

What do you think of this?

@iceman1001
Copy link
Member

As long as we have one way of stubbing printandlog function, on device, or on android, simplifying the code, in one place I'm fine. If it needs to go to a seperate file for it to happend, I see no issue with it.
Indeed, a nice abstract implementation would be helpful.

it can not be spread over several files, with defines, or execptions, making codesmells and hard to maintain scenarios. iclass commmands uses one, you use one.

I looking forward to see a good impl and you can close / create a new PR.

@pwpiwi
Copy link
Contributor

pwpiwi commented Dec 27, 2017

I don't see an issue with this PR. I don't think that the #define prnt PrintAndLog has a specific reason.

@marshmellow42
Copy link
Contributor

If you are referring to the define prnt PrintAndLog in lfdemod.c, it is there only to allow that file to be used on arm, where there is no PrintAndLog, and client. Which is a different purpose than this PR.

@pwpiwi
Copy link
Contributor

pwpiwi commented Dec 28, 2017

common/protocols.c

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.

4 participants