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

[Windows] Fixed ppi_geotag #479

Merged
merged 2 commits into from
Feb 15, 2017
Merged

Conversation

gpotter2
Copy link
Member

Fixes ppi_geotag's part in #432 + added a few tests

@codecov-io
Copy link

codecov-io commented Jan 20, 2017

Current coverage is 69.48% (diff: 100%)

Merging #479 into master will decrease coverage by 0.10%

@@             master       #479   diff @@
==========================================
  Files           104        106     +2   
  Lines         25622      25938   +316   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          17831      18023   +192   
- Misses         7791       7915   +124   
  Partials          0          0          
Diff Coverage File Path
•••••••••• 100% scapy/contrib/ppi_geotag.py

Powered by Codecov. Last update 9c8f1a7...d71e28c

@p-l-
Copy link
Member

p-l- commented Jan 20, 2017

Could you explain why you replaced time.gmtime(0)?

@gpotter2
Copy link
Member Author

gpotter2 commented Jan 21, 2017

It's a windows bug: time.gmtime(0) will return a tuple with the date (1970, 1, 1). But for some reasons, on windows only, the range starts at (1970,1,2)

See the error stack trace at #432

@p-l-
Copy link
Member

p-l- commented Jan 21, 2017

@gpotter2 so maybe you could just define a global constant EPOCH = time.mktime((1970, 1, 1, 0, 0, 0, 0, 0, 0))? BTW I think the current code is wrong, because you have used 1970-1-2 as the start time, while it should be 1970-1-1.

@gpotter2
Copy link
Member Author

@p-l- The windows bug: time.mktime((1970, 1, 1, 0, 0, 0, 0, 0, 0)) crash because Windows Epoch is 01/01/1970 at 00:00 but of the next day (so 01/02/1970 at 00:00)
time.mktime((1970, 1, 2, 0, 0, 0, 0, 0, 0))-86400 will do the trick i guess...

@gpotter2
Copy link
Member Author

That should do the trick

@gpotter2 gpotter2 force-pushed the fix-modules-windows branch from 5417c73 to d71e28c Compare January 21, 2017 17:50
@gpotter2 gpotter2 changed the title [Tiny Fixes/Windows] Fixed ppi_geotag [Windows] Fixed ppi_geotag Jan 22, 2017
@guedou guedou merged commit 21a0cb1 into secdev:master Feb 15, 2017
@gpotter2
Copy link
Member Author

@guedou Hey, on master this if failing... I don't know what has changed since the tests were performed... any ideas ?

@guedou
Copy link
Member

guedou commented Feb 15, 2017

@gpotter2 I have no idea =/ The patches looked good to me.

The issue might be related to PR#471 Could you have a look ?

@gpotter2
Copy link
Member Author

gpotter2 commented Feb 15, 2017

I'll check that
@p-l CAN you have a look too, please?
https://travis-ci.org/secdev/scapy/jobs/201869982#L7615

Edit: ok i've found the bug. Not sure what created it but #515 fixes it

@gpotter2 gpotter2 deleted the fix-modules-windows branch February 22, 2017 21:34
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