v2: do not create uninitialized sway_keyboard

[Hi-Angel]

It's not good to create a uninitialized struct, because it might become
a possible error in future. I actually got hit by this while working on
#3155

v2: just move initialization inside the keyboard creation, leave the rest in place.

[emersion]

I don't get this change. In both cases, sway_keyboard_configure is already called. Also, there's no uninitialized struct because we use calloc everywhere.

[Hi-Angel]

calloc can't initialize struct fields correctly. For example, there was some pointer which was set to zero, whilst in properly configured keyboard it should've pointed to something, which led to a crash for me until I figured that just after sway_keyboard_create() finished execution, the sway_keyboard is still partially uninitialized.

This change makes sure that if we ever created a sway_keyboard, it's properly initialized.

upd: s/uninitialized/partially uninitialized

[Hi-Angel]

There's also a more general point (it doesn't immediately apply to this situation though) is that when you have two separate calls for struct creation and initialization, you need to create a temporary variable, and pass it between two calls. Often such design also makes problems with constifying variables.

[ghost]

calloc can't initialize struct fields correctly. For example, there was some pointer which was set to zero, whilst in properly configured keyboard it should've pointed to something

Seems like you're using overloaded terminology here. An uninitialized struct will have fields of indeterminate value. calloc does initialize the struct with all zeroes, so the struct is already initialized. However, it sounds like your problem is you tried to dereference a pointer before it pointed to anything. What were you trying to dereference between calls of sway_keyboard_create and sway_keyboard_configure?

[Hi-Angel]

Seems like you're using overloaded terminology here. An uninitialized struct will have fields of indeterminate value. calloc does initialize the struct with all zeroes, so the struct is already initialized.

Ok, maybe I do. Although I'd argue there's little difference between a random value assigned to a field (i.e. uninitialized), and a wrong value assigned to the same field. But sure, a field initialized to wrong value is still initialized.

Still, terminology doesn't make this pull request less useful.

However, it sounds like your problem is you tried to dereference a pointer before it pointed to anything. What were you trying to dereference between calls of sway_keyboard_create and sway_keyboard_configure?

This line:

keyboard->default_kbd_layout =
 		sway_keyboard_get_layout(wlr_device->keyboard->xkb_state);

The xkb_state is zero between the calls.

[ghost]

Is there any reason you can't move that code after wlr_keyboard_set_keymap? Adding more calls to sway_keyboard_configure may or may not have additional side effects.

[Hi-Angel]

Is there any reason you can't move that code after wlr_keyboard_set_keymap? Adding more calls to sway_keyboard_configure may or may not have additional side effects.

I kind of did, the diff I referred to in my prev. comment have this code after the wlr_keyboard_set_keymap(). Or do you mean, like, put it right after the call? (I put it to the end of the sway_keyboard_configure())?

[Hi-Angel]

@remyabel I think you misunderstood me. The code I referred to works right now, I just showed the line that per my memory resulted in crash back when the line was inside the sway_keyboard_create call.