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

adding compile time to PE binaries #2177

Closed
wants to merge 3 commits into from
Closed

Conversation

gssincla
Copy link
Contributor

No description provided.

@radare
Copy link
Collaborator

radare commented Mar 12, 2015

R2 have code to parse dates, shouldnt we store a human readable form here?

On 12 Mar 2015, at 04:27, gssincla [email protected] wrote:

You can view, comment on, or merge this pull request online at:

#2177

Commit Summary

adding compile time to PE binaries
File Changes

M libr/bin/format/pe/pe.c (4)
Patch Links:

https://github.com/radare/radare2/pull/2177.patch
https://github.com/radare/radare2/pull/2177.diff

Reply to this email directly or view it on GitHub.

@gssincla
Copy link
Contributor Author

by leaving it in the integer format you avoid a few problems for analysts who want to use r2 for automated analysis:

  1. you don't have to parse a text representation back to an integer
  2. you don't lock the resulting time into a specific time zone (some compilers seem to use UTC while others seem to use the compiler's local time... so an analysis has to make that call as to what the real time is). Using a string date would require the analyst to do additional work to adjust TZ.

If a string date is desired, i could always go back and add a second entry like TimeStamp_string with the date parsed to a string in the UTC timezone.

@radare
Copy link
Collaborator

radare commented Mar 12, 2015

yep. i would prefer to have two entries, one for humans and another for scripting

i think this way it’s better for everyone :)

also r2 timestamping doesnt supports timezones, it’s in the TODO, and i missed that feature a couple of times, but end up doing the time change by hand when i needed it. Would you want to do this too?

The idea would be to add another config var in e to define the timezone. and in case the bin file contains timezone information set it from libr/core/bin.c

On 12 Mar 2015, at 14:08, gssincla [email protected] wrote:

by leaving it in the integer format you avoid a few problems for analysts who want to use r2 for automated analysis:

  1. you don't have to parse a text representation back to an integer
  2. you don't lock the resulting time into a specific time zone (some compilers seem to use UTC while others seem to use the compiler's local time... so an analysis has to make that call as to what the real time is). Using a string date would require the analyst to do additional work to adjust TZ.

If a string date is desired, i could always go back and add a second entry like TimeStamp_string with the date parsed to a string in the UTC timezone.


Reply to this email directly or view it on GitHub #2177 (comment).

@gssincla
Copy link
Contributor Author

added the string timestamp. didnt add timezone. doing a new pullreq

@Maijin
Copy link
Contributor

Maijin commented Mar 12, 2015

#921

@radare
Copy link
Collaborator

radare commented Mar 13, 2015

Cool! squashed and picked!

@radare radare closed this Mar 13, 2015
@radare
Copy link
Collaborator

radare commented Mar 13, 2015

Also, time_t != long. it must be casted at least to avoid a warning.

see my last commit

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