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

A remedy to #217 #221

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

A remedy to #217 #221

wants to merge 17 commits into from

Conversation

deepfire
Copy link

@deepfire deepfire commented Mar 27, 2017

This is an attempt to remedy #217.

Copy link
Contributor

@plohrmann plohrmann left a comment

Choose a reason for hiding this comment

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

Aside from the one comment, looks good to me. Since the behavior of vogleditor would change slightly (although I doubt anyone will care about the limitation), can you add add a line to the readme to identify the change in behavior?

@@ -471,7 +471,7 @@ VoglEditor::Prompt_Result VoglEditor::prompt_generate_trace()
#endif

QString cmdLine = m_pLaunchTracerDialog->get_command_line();
QProcessEnvironment env = m_pLaunchTracerDialog->get_process_environment();
QProcessEnvironment env = m_pLaunchTracerDialog->get_process_environment (sizeof (void *) < 8);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it's particularly common, but this means a 64-bit vogleditor could not launch and trace a 32-bit application, because you're only going to inject the 64-bit tracer.

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely. The problem is, leaving this as-is was breaking the tracing in my case.

Copy link
Author

Choose a reason for hiding this comment

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

The true answer to this question, IMO, is figuring out how to build a separate, 32-bit tracer using Nix.

Choose a reason for hiding this comment

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

@plohrmann, I have addressed your suggestion (I think)

@deepfire
Copy link
Author

@plohrmann, done!

@deepfire deepfire force-pushed the master branch 2 times, most recently from c617f4c to 7c12134 Compare April 1, 2017 08:12
@@ -348,7 +348,7 @@ bool vogleditor_QApiCallTreeModel::init(vogl_trace_file_reader *pTrace_reader)
{
QString msg(QString("*** Information: unpaired \"") + QString(g_vogl_entrypoint_descs[entrypoint_id].m_pName) + QString("\"."));
vogleditor_output_message(msg.toStdString().c_str());
vogl_printf(msg.toStdString().c_str());
vogl_printf("%s", msg.toStdString().c_str());
vogl_printf("\n");

Choose a reason for hiding this comment

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

can be merged with the previous line "%s\n"

Choose a reason for hiding this comment

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

@mgerhardy, done

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