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

Prevent invalid function parameter order on async callback by not enforcing it #814

Merged
merged 1 commit into from Jul 18, 2019

Conversation

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 17, 2019

Fixes #813.

Can you check it @Boiethios please?

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jul 18, 2019

@GuillaumeGomez PR broke gio, now many errors like this:

error[E0061]: this function takes 2 parameters but 1 parameter was supplied
   --> D:/eap/rust/0/gio\src\auto\app_info.rs:144:21
    |
144 |             let _ = gio_sys::g_app_info_launch_default_for_uri_finish(&mut error);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 2 parameters

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-args-order-async branch from 48c6ff0 to 2888aff Jul 18, 2019
@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Jul 18, 2019

Updated.

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-args-order-async branch from 2888aff to 25e0f4f Jul 18, 2019
@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Jul 18, 2019

Added the assert.

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-args-order-async branch from 25e0f4f to 3a9ae98 Jul 18, 2019
@Boiethios

This comment has been minimized.

Copy link

Boiethios commented Jul 18, 2019

That works well. The generated code for GOA compiles correctly 👌

@Boiethios

This comment has been minimized.

Copy link

Boiethios commented Jul 18, 2019

There is just a small thing: the return is not used, and that triggers a warning:

warning: unused variable: `ret`
   --> src/auto/account.rs:246:17
    |
246 |             let ret = goa_sys::goa_account_call_ensure_credentials_finish(_source_object as *mut _, out_expires_in.as_mut_ptr(), res, &mut error);
    |                 ^^^ help: consider prefixing with an underscore: `_ret`
    |
    = note: #[warn(unused_variables)] on by default
@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Jul 18, 2019

That's an actual bug I guess. The out_expires_in is only valid if true was returned. And if false was returned we need to handle the error.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Jul 18, 2019

Should I fix it in here directly?

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Jul 18, 2019

Sure

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:fix-args-order-async branch from 3a9ae98 to 0ab7700 Jul 18, 2019
@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Jul 18, 2019

@Boiethios Try again? :)

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Jul 18, 2019

@GuillaumeGomez Thanks, please merge yourself when @Boiethios accepts

@Boiethios

This comment has been minimized.

Copy link

Boiethios commented Jul 18, 2019

That seems to be ok. Here is the generated code:

fn call_ensure_credentials<P: IsA<gio::Cancellable>, Q: FnOnce(Result<i32, Error>) + Send + 'static>(&self, cancellable: Option<&P>, callback: Q) {
    let user_data: Box<Q> = Box::new(callback);
    unsafe extern "C" fn call_ensure_credentials_trampoline<Q: FnOnce(Result<i32, Error>) + Send + 'static>(_source_object: *mut gobject_sys::GObject, res: *mut gio_sys::GAsyncResult, user_data: glib_sys::gpointer) {
        let mut error = ptr::null_mut();
        let mut out_expires_in = mem::MaybeUninit::uninit();
        let _ = goa_sys::goa_account_call_ensure_credentials_finish(_source_object as *mut _, out_expires_in.as_mut_ptr(), res, &mut error);
        let out_expires_in = out_expires_in.assume_init();
        let result = if error.is_null() { Ok(out_expires_in) } else { Err(from_glib_full(error)) };
        let callback: Box<Q> = Box::from_raw(user_data as *mut _);
        callback(result);
    }
    let callback = call_ensure_credentials_trampoline::<Q>;
    unsafe {
        goa_sys::goa_account_call_ensure_credentials(self.as_ref().to_glib_none().0, cancellable.map(|p| p.as_ref()).to_glib_none().0, Some(callback), Box::into_raw(user_data) as *mut _);
    }
}

If I understand correctly the code, the return value is discarded since it only indicates if error has been set.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

GuillaumeGomez commented Jul 18, 2019

Ok, then I merge!

@GuillaumeGomez GuillaumeGomez merged commit 22b8475 into gtk-rs:master Jul 18, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:fix-args-order-async branch Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.