Re: Review Request 112294: Implement multi-seat support in KDM

2014-06-02 Thread Oswald Buddenhagen
On May 31, 2014, 9:35 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1463 https://git.reviewboard.kde.org/r/112294/diff/8/?file=276202#file276202line1463 this is actually tricky ... what if the display is still there but the seat changed? i don't think you are handling

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Oswald Buddenhagen
On May 26, 2014, 8:32 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1436 https://git.reviewboard.kde.org/r/112294/diff/7/?file=274916#file274916line1436 no matching unused Stefan Brüns wrote: There is no unused or no matching unused dynamic display for %s sounds

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review58845 --- kdm/backend/dm.c

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Stefan Brüns
On May 31, 2014, 9:35 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1416 https://git.reviewboard.kde.org/r/112294/diff/8/?file=276202#file276202line1416 that should go into the if for clarity. more importantly, you also need an unregisterInput() and a

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Stefan Brüns
On May 26, 2014, 8:32 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1436 https://git.reviewboard.kde.org/r/112294/diff/7/?file=274916#file274916line1436 no matching unused Stefan Brüns wrote: There is no unused or no matching unused dynamic display for %s sounds

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Stefan Brüns
On May 31, 2014, 9:35 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1463 https://git.reviewboard.kde.org/r/112294/diff/8/?file=276202#file276202line1463 this is actually tricky ... what if the display is still there but the seat changed? i don't think you are handling

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-31 Thread Oswald Buddenhagen
On May 31, 2014, 9:35 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1463 https://git.reviewboard.kde.org/r/112294/diff/8/?file=276202#file276202line1463 this is actually tricky ... what if the display is still there but the seat changed? i don't think you are handling

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-29 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/client.c, line 1467 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186610#file186610line1467 this is redundant with the line below. if some particular pam module needs this setting, it should be done

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-29 Thread Stefan Brüns
On May 26, 2014, 2:50 p.m., Stefan Brüns wrote: kdm/backend/dm.c, line 1430 https://git.reviewboard.kde.org/r/112294/diff/7/?file=274916#file274916line1430 Hm, I prefer this notation, in particular as this is the same as for std::string (guaranteed since C++03, working for all

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-29 Thread Stefan Brüns
On May 24, 2014, 11:46 a.m., Oswald Buddenhagen wrote: kdm/config.def, line 838 https://git.reviewboard.kde.org/r/112294/diff/6/?file=273437#file273437line838 this should come before the ReserveServers - as mentioned before, hypothetically, you could have reserve displays for

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-29 Thread Stefan Brüns
On May 26, 2014, 8:32 a.m., Oswald Buddenhagen wrote: kdm/backend/client.c, line 1465 https://git.reviewboard.kde.org/r/112294/diff/7/?file=274914#file274914line1465 you forgot the free(envbuf). i don't like the interminglig with the other type of setting env vars.

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-29 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated May 29, 2014, 7:03 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-28 Thread Oswald Buddenhagen
On May 26, 2014, 2:50 p.m., Stefan Brüns wrote: kdm/backend/dm.c, line 1430 https://git.reviewboard.kde.org/r/112294/diff/7/?file=274916#file274916line1430 Hm, I prefer this notation, in particular as this is the same as for std::string (guaranteed since C++03, working for all

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-26 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review58442 --- kdm/backend/client.c

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-26 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review58481 --- kdm/backend/dm.h

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-25 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated May 26, 2014, 12:06 a.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-25 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review58428 --- kdm/backend/client.c

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-24 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/#review58394 --- i didn't finish the review, because it seems that revision 6

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-24 Thread Stefan Brüns
On May 24, 2014, 11:46 a.m., Stefan Brüns wrote: regarding DynamicDisplays and your magic backwards compat handling ... i think the seat-encoded config stuff i insisted on makes that redundant, after all. suppose we do it like that: StaticServers=:0,@seat1_:4,@seat2_:7

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-18 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated May 18, 2014, 7:44 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-18 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1397 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397 that seems questionable to me. why are you re-defining the display to be permanent? when the seat goes away, kdm

Re: Review Request 112294: Implement multi-seat support in KDM

2014-05-18 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/client.c, line 1467 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186610#file186610line1467 this is redundant with the line below. if some particular pam module needs this setting, it should be done

Re: Review Request 112294: Implement multi-seat support in KDM

2014-04-09 Thread Oswald Buddenhagen
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1397 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397 that seems questionable to me. why are you re-defining the display to be permanent? when the seat goes away, kdm

Re: Review Request 112294: Implement multi-seat support in KDM

2014-04-07 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1409 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1409 i prefer !strcmp(). repeats several times. also, this seems like a hack. it should be possible to

Re: Review Request 112294: Implement multi-seat support in KDM

2014-04-06 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated April 6, 2014, 8:16 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-04-06 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/server.c, line 85 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186613#file186613line85 is this really necessary? i would expect it to be the default. Its not necessary (and the code path in the xserver --

Re: Review Request 112294: Implement multi-seat support in KDM

2014-04-06 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated April 6, 2014, 10:01 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-31 Thread Oswald Buddenhagen
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1397 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397 that seems questionable to me. why are you re-defining the display to be permanent? when the seat goes away, kdm

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-30 Thread Oswald Buddenhagen
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1351 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1351 you can leave out the automatic multiseat won't be enabled from the followup messages. isn't there a

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-29 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated March 29, 2014, 5:14 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-29 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/112294/ --- (Updated March 29, 2014, 5:38 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-29 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1351 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1351 you can leave out the automatic multiseat won't be enabled from the followup messages. isn't there a

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-29 Thread Stefan Brüns
On March 28, 2014, 10:59 a.m., Oswald Buddenhagen wrote: kdm/backend/dm.c, line 1397 https://git.reviewboard.kde.org/r/112294/diff/2/?file=186612#file186612line1397 that seems questionable to me. why are you re-defining the display to be permanent? when the seat goes away, kdm

Re: Review Request 112294: Implement multi-seat support in KDM

2014-03-27 Thread Stefan Brüns
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2014-02-18 Thread Aaron J. Seigo
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2014-02-18 Thread Oswald Buddenhagen
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2014-02-18 Thread Aaron J. Seigo
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-06 Thread Oswald Buddenhagen
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-05 Thread Oswald Buddenhagen
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-05 Thread Martin Tobias Holmedahl Sandsmark
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? Stefan Brüns wrote: The

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-04 Thread Stefan Brüns
On Sept. 3, 2013, 10:20 p.m., Oswald Buddenhagen wrote: given that there is no intention to make further feature releases of the kde workspace which will include kdm, i wonder why we'd go through the (potentially tedious) process of upstreaming this now? The reason for sending this was

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-03 Thread Oswald Buddenhagen
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review39302 --- given that there is no intention to make further feature

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-02 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- (Updated Sept. 2, 2013, 7:57 p.m.) Review request for kde-workspace.

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-02 Thread Martin Tobias Holmedahl Sandsmark
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review39221 --- I don't see anything obviously wrong with it, but you should

Re: Review Request 112294: Implement multi-seat support in KDM

2013-09-02 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- (Updated Sept. 2, 2013, 11:34 p.m.) Review request for kde-workspace and

Re: Review Request 112294: Implement multi-seat support in KDM

2013-08-27 Thread Martin Tobias Holmedahl Sandsmark
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review38743 --- kdm/backend/dm.h

Re: Review Request 112294: Implement multi-seat support in KDM

2013-08-27 Thread Stefan Brüns
On Aug. 27, 2013, 2:08 p.m., Stefan Brüns wrote: looks fine to me, just that tiny minor nitpick on variable naming. I assume this applies to systemd_monitor{,_fd} in dm.c as well. Will fix, waiting a few days for more comments ... - Stefan

Re: Review Request 112294: Implement multi-seat support in KDM

2013-08-27 Thread Martin Bříza
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/#review38749 --- Hi, although I'm not entirely convinced this patch's quality

Review Request 112294: Implement multi-seat support in KDM

2013-08-26 Thread Stefan Brüns
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112294/ --- Review request for kde-workspace. Description --- This patch