On Tue, 2009-10-27 at 10:50 -0400, Tony Espy wrote: > On 10/27/2009 10:29 AM, Jirka Klimes wrote: > > On Tuesday 27 October 2009 15:04:51 Tony Espy wrote: > > > >> On 10/27/2009 07:58 AM, Jirka Klimes wrote: > >> > >>> On Friday 23 October 2009 19:19:14 Dan Williams wrote: > >>> > >>>> Anyone want to do a really simple patch? It would close some bugs and > >>>> help out a lot: > >>>> > >>> Hello Dan, > >>> > >>> I've done the patch. Please, review it. > >>> > >> Jirka -- > >> > >> You beat me to the punch... I finished up my version last night, but > >> hadn't tested it yet. > >> > >> > > Sorry for that. I hope you didn't spend much time on it. > > Nevertheless it's good when more people check the stuff. > > > > No worries... > > >> You version is actually a little bit cleaner, as I hadn't created a new > >> struct for the config parameters. I've attached my version ( which is > >> actually a quilt patch for Ubuntu ) for reference. > >> > >> The one thing I think you may have missed, is checking for > >> G_KEY_FILE_ERROR_INVALID_VALUE when reading the values from the file. > >> Other than that, looks good to me! > >> > >> > > I'm aware of G_KEY_FILE_ERROR_INVALID_VALUE. However I decided not to > > produce > > errors for reading the values. > > So: > > missing keys - regarded as TRUE > > erroneous values - regarded as FALSE > > > > Well, the only problem I see is that a bad value means you could end up > with networking and/or wireless disabled. > > That said, at least with your approach, a user can restore one or both > via the applet, whereas if we return an error, the user has to figure > out how to restart NetworkManger. > > If we decide to stick with your approach, perhaps a log message is > warranted? > > One last thing that's been eating at me a little, is that we're saving > program state to a configuration file. As such, we could potentially > lose state if we decide to drop a new configuration file into place in a > future update. > > [ Dan - any thoughts on creating a separate .state file instead? ]
Yeah, that's probably valid. I wonder where to put it though, since we want this to be preserved across reboot, and /var/run or /var/lib isn't guaranteed to be so, right? That's usually what /etc is for. Or are you suggesting /etc/NetworkManager.state ? Dan _______________________________________________ NetworkManager-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/networkmanager-list
