GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix colormaps by kryptan · Pull Request #92 · arrayfire/arrayfire-rust

Conversation

kryptan commented Nov 15, 2016

Copy link

Copy Markdown

Contributor

Colormaps are not currently working because layout of the Cell structure doesn't match its version in arrayfire. Also there are some potential bugs because Rust's non null-terminated strings were passed to C.

This pull request fixes this. I also simplified code in graphics.rs where other strings were passed to C.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

kryptan added 2 commits

November 15, 2016 22:28


          Simplify string passing between Rust and C

32f0868


          Fix Cell structure layout

576fa0e

9prady9 requested changes

Nov 17, 2016

View reviewed changes

9prady9 left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Except the change i requested in a separate comment, everything else looks good. Thank you for the contributions!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/graphics.rs Outdated

		let xstr = CString::new(xlabel.as_bytes()).unwrap();
		let ystr = CString::new(ylabel.as_bytes()).unwrap();
		let zstr = CString::new(zlabel.as_bytes()).unwrap();
		let cprops = &Cell {row: self.row, col: self.col, title: [0].as_ptr(), cmap: self.cmap};

9prady9 Nov 17, 2016

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you please use std::ptr::null instead of an static single element slice to get a invalid pointer. Please do similar change in all other places where [0].as_ptr() is being used.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Use null pointer instead of a pointer to an empty string

9c35ce4

9prady9 approved these changes

Nov 17, 2016

View reviewed changes

9prady9 merged commit babec9d into arrayfire:devel

Nov 17, 2016

Reviewers

9prady9

9prady9 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.