> > First patch: > > - I agree with the structuring of the xrandr part, good! > > - But not with the localization of every data, they're used across > > the whole program and you're just over-complicating functions here > > imho. The program is simple and clear enough not to have to get > > triple pointers passed as function parameters. > > the triple pointer is just used once in the cleanup function, as after > freeing the locks we set the locks double pointer to NULL. That was just one example of the rest of the complexity it brings in here.
> > - un-typedef, that's not an issue here, why changing it besides > > satisfying your personal taste? > > because it would be inconsistent with the xrandr-struct. It would be inconsistent with… An inconsistency you're bringing in? ^^ > There is also > no reason to typedef struct anyway and just hides the underlying > machinery for no good reason. As I said, personal taste. > > Second patch: > > - fine by me, renaming variables patch ;p > > You wouldn't believe how much difference it makes to improve variable > naming. No I wouldn't! I have no clue! > > Third patch: > > - the rval renaming belongs to the second patch > > It's debatable if you think of the second patch as a renaming patch, Is it not? Does it make more sense to have it in a patch which changes where we get the user login from? > but in the end I don't see it as a big issue. Not really indeed (either way). > > - I agree with the removal of the $USER, we discussed it before this > > patch on IRC. But maybe we should have the reasoning behind that > > from the original author who put it here. > > The reasoning is that we don't want a bloody seduid program depending > so strongly on environment variables. For some things, it's sufficient > to just kick them out because they are so obviously wrong. Not *that* reasoning, as I already said, I already told you I was for getting rid of it. I mean the reasoning of why it was introduced in the first place, to be sure something is not missed here. > What has been bugging me for quite a while is this DPMS comment that > was added there for no reason. […] Get rid of it.
