On Sun, Dec 14, 2014 at 05:44:11PM +0000, Sami Kerola wrote:
> + ctl->newf.full_name = prompt(_("Name"), ctl->oldf.full_name);
> + ctl->newf.office = prompt(_("Office"), ctl->oldf.office);
> + ctl->newf.office_phone = prompt(_("Office Phone"), ctl->oldf.office_phone);
> + ctl->newf.home_phone = prompt(_("Home Phone"), ctl->oldf.home_phone);
it would be better to rename prompt() to ask_new_field() or so.
> -static int set_changed_data(struct finfo *oldfp, struct finfo *newfp)
> +static int set_changed_data(struct chfn_control *ctl)
> {
> int changed = false;
>
> - if (newfp->full_name) {
> - oldfp->full_name = newfp->full_name;
> + if (ctl->newf.full_name)
> changed = true;
> - }
> - if (newfp->office) {
> - oldfp->office = newfp->office;
> + else
> + ctl->newf.full_name = ctl->oldf.full_name;
> + if (ctl->newf.office)
> changed = true;
> - }
> - if (newfp->office_phone) {
> - oldfp->office_phone = newfp->office_phone;
> + else
> + ctl->newf.office = ctl->oldf.office;
> + if (ctl->newf.office_phone)
> changed = true;
> - }
> - if (newfp->home_phone) {
> - oldfp->home_phone = newfp->home_phone;
> + else
> + ctl->newf.office_phone = ctl->oldf.office_phone;
> + if (ctl->newf.home_phone)
> changed = true;
> - }
> -
> + else
> + ctl->newf.home_phone = ctl->oldf.home_phone;
> return changed;
> }
this is ugly and unnecessary function, all you need is to return
usable data from prompt() and maintain ctl->changed with in the
function. The prompt() knows all the detains.
> /* create the new gecos string */
> - len = xasprintf(&gecos, "%s,%s,%s,%s,%s", pinfo->full_name, pinfo->office,
> - pinfo->office_phone, pinfo->home_phone, pinfo->other);
> + len = xasprintf(&gecos, "%s,%s,%s,%s,%s", ctl->newf.full_name, ctl->newf.office,
> + ctl->newf.office_phone, ctl->newf.home_phone, ctl->newf.other);
Maybe
len = xasprintf(&gecos, "%s,%s,%s,%s,%s",
ctl->newf.full_name ? ctl->newf.full_name : "",
...
);
would be better than set the empty strings to newf (now you mix ""
and strdup() results in newf).
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
