dhcpcd icon indicating copy to clipboard operation
dhcpcd copied to clipboard

Option 2: Fix stdin parsing

Open holmanb opened this issue 1 year ago • 4 comments

This is my second fix https://github.com/NetworkConfiguration/dhcpcd/issues/285. It checks if there was a single "interface" argument passed and if that argument value is -. If such an argument exists, then dhcpcd forcibly waits on stdin. This also removes the unreliable "check if the stdin buffer is full" semantics, which I don't think should break other use cases. Running dhcpcd -U <interface> will work just like it did before, by contacting the network manager[1]. I assume you prefer this fix over the first one I wrote, because that one breaks the "no interface means dump all interfaces" expectation, but I still offered both fixes for you to decide because that documented expectation also appears broken.

[1] this requirement also seems like a bug when the lease file has already been acquired with a --oneshot call, but as long as we have some stable way across releases to parse a lease without the daemon running I guess I don't care strongly if that one of these gets fixed

holmanb avatar Jan 20 '24 02:01 holmanb

Aside: Apologies for the flurry of similar issues filed and multiple PRs for the same issue. I'm currently trying to document what I see as unexpected / broken behavior, and (for now) fix the ones that I see as most important (https://github.com/NetworkConfiguration/dhcpcd/issues/285 and also https://github.com/NetworkConfiguration/dhcpcd/issues/282, which I see now has a PR waiting review: https://github.com/NetworkConfiguration/dhcpcd/pull/284).

holmanb avatar Jan 20 '24 02:01 holmanb

You'll need to change dhcpcd.8.in as well to document the new requirement for dumping leases needing the - part. Luckily interface names cannot start with a - so this is a safe change.

rsmarples avatar Feb 16 '24 16:02 rsmarples

Thanks for the review @rsmarples. I'm busy now, but will try to get back to this PR in the next few weeks if possible. Otherwise I plan circle back to this after the next Ubuntu release.

holmanb avatar Mar 11 '24 15:03 holmanb

@holmanb Has this made any progress?

perkelix avatar May 07 '24 14:05 perkelix

@holmanb @perkelix I decided to merge it and and adjust it in 43954779. It's a good idea.

rsmarples avatar Jun 17 '24 09:06 rsmarples