Skip to content

Conversation

@ebugden
Copy link
Owner

@ebugden ebugden commented Jul 20, 2017

Feedback concerning the completeness and the accuracy of the tests would be much appreciated.

Also maybe suggestions for how to make the error management in the Python more pythonic.

# Determine the address of the probe.
# The address is always found at the same place with reference to
# the start of the probe provider.
addr_start_idx = probe_index-(2*8*3)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please explain those numbers in a comment or declare them as variables


def main():
# Import libsdt-offset
current_dir = os.path.abspath(os.path.dirname(__file__))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we use os.getcwd() to get the current working directory?
https://docs.python.org/3/library/os.html#os.getcwd

import sys
import subprocess
import os.path
import collections
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we only need namedtuple, let's use this format: from collections import namedtuple.

function_name_bytes = None

offset = lib.elf_get_function_offset(fd,
c_char_p(function_name_bytes))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Align with first argument of the fonction

c_char_p(probe_name_byte_str))

if offset == test1.expected_result:
print(str(test1.test_num) + " - " + test1.test_name + ": pass\n")
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets use single quote everywhere to define strings


if process.returncode != 0:
print('Invalid file path or no .text section in binary.')
return -1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using exceptions here, would be more pythonic

# Use objdump (command line tool) to get .text ELF section header
# info.
process = subprocess.Popen(
['objdump', '--headers', '--section=.text', file_path],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you define this list in variable out of the function call?

def get_sdt_note_section_hex_str(file_path, sdt_note_section):
# Use objdump (command line tool) to retrieve section header.
process = subprocess.Popen(
['objdump', '--headers', '--section=.note.stapsdt', file_path],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Align with the function call.

import re
import binascii
from ctypes import *

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, explain our definition of offset and address and why we need this function.

Makefile is for building the binaries necessary for testing the library.

Signed-off-by: Erica Bugden <erica.bugden@efficios.com>
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