More pdb_ldap.c stuff, a question of style

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Mar 24 18:06:12 GMT 2003


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi!

As I said in my commit message, I hate to pass down boolean flags to
inner functions to trigger something magic. Thus I had the idea to
pass down a function that decides whether to update the LDAP DB. This
way the complete logic is lexically much closer together. C syntax
does not make that nice, but we have to live with that.

What do you think about that (completely untested) patch?

Volker

P.S: If we only had a *real* programming language at hand... Anybody
about to re-write Samba in ML or so? ;-))

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: Key-ID D32186CF, Fingerprint available: phone +49 551 3700000

iD8DBQE+f0jNOmSXH9Mhhs8RApUKAJ9jkTUJAmRArSdcgfx7CGK47CvhAQCeJdoy
MKrt2s6i9bwM9GdNm5Es39k=
=cZCv
-----END PGP SIGNATURE-----

Index: pdb_ldap.c
===================================================================
RCS file: /data/cvs/samba/source/passdb/pdb_ldap.c,v
retrieving revision 1.93
diff -u -r1.93 pdb_ldap.c
--- pdb_ldap.c	23 Mar 2003 14:19:13 -0000	1.93
+++ pdb_ldap.c	24 Mar 2003 18:00:42 -0000
@@ -1292,13 +1292,15 @@
 *********************************************************************/
 static void make_ldap_mod(LDAP *ldap_struct, LDAPMessage *existing,
 			  LDAPMod ***mods,
-			  const SAM_ACCOUNT *sampass, BOOL pdb_add,
+			  const SAM_ACCOUNT *sampass,
+			  BOOL (*need_update)(const SAM_ACCOUNT *,
+					      enum pdb_elements),
 			  enum pdb_elements element,
 			  const char *attribute, const char *newval)
 {
 	char **values = NULL;
 
-	if (!need_ldap_mod(pdb_add, sampass, element)) {
+	if (!need_update(sampass, element)) {
 		return;
 	}
 
@@ -1348,7 +1350,8 @@
 static BOOL init_ldap_from_sam (struct ldapsam_privates *ldap_state, 
 				LDAPMessage *existing,
 				LDAPMod *** mods, const SAM_ACCOUNT * sampass,
-				BOOL pdb_add)
+				BOOL (*need_update)(const SAM_ACCOUNT *,
+						    enum pdb_elements))
 {
 	pstring temp;
 	uint32 rid;
@@ -1364,7 +1367,7 @@
 	 * took out adding "objectclass: sambaAccount"
 	 * do this on a per-mod basis
 	 */
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_USERNAME, "uid", pdb_get_username(sampass));
 	DEBUG(2, ("Setting entry for user: %s\n", pdb_get_username(sampass)));
 
@@ -1392,7 +1395,7 @@
 	}
 
 	slprintf(temp, sizeof(temp) - 1, "%i", rid);
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_USERSID, "rid", temp);
 
 
@@ -1412,7 +1415,7 @@
 	}
 
 	slprintf(temp, sizeof(temp) - 1, "%i", rid);
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_GROUPSID, "primaryGroupID", temp);
 
 	/* displayName, cn, and gecos should all be the same
@@ -1423,55 +1426,55 @@
 	 *  it does not exist.
 	 */
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_FULLNAME, "displayName",
 		      pdb_get_fullname(sampass));
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_ACCTDESC, "description",
 		      pdb_get_acct_desc(sampass));
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_WORKSTATIONS, "userWorkstations",
 		      pdb_get_workstations(sampass));
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_SMBHOME, "smbHome",
 		      pdb_get_homedir(sampass));
 			
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_DRIVE, "homeDrive",
 		      pdb_get_dir_drive(sampass));
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_LOGONSCRIPT, "scriptPath",
 		      pdb_get_logon_script(sampass));
 
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_PROFILE, "profilePath",
 		      pdb_get_profile_path(sampass));
 
 	slprintf(temp, sizeof(temp) - 1, "%li", pdb_get_logon_time(sampass));
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_LOGONTIME, "logonTime", temp);
 
 	slprintf(temp, sizeof(temp) - 1, "%li", pdb_get_logoff_time(sampass));
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_LOGOFFTIME, "logoffTime", temp);
 
 	slprintf (temp, sizeof (temp) - 1, "%li",
 		  pdb_get_kickoff_time(sampass));
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_KICKOFFTIME, "kickoffTime", temp);
 
 	slprintf (temp, sizeof (temp) - 1, "%li",
 		  pdb_get_pass_can_change_time(sampass));
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_CANCHANGETIME, "pwdCanChange", temp);
 
 	slprintf (temp, sizeof (temp) - 1, "%li",
 		  pdb_get_pass_must_change_time(sampass));
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_MUSTCHANGETIME, "pwdMustChange", temp);
 
 	if ((pdb_get_acct_ctrl(sampass)&(ACB_WSTRUST|ACB_SVRTRUST|ACB_DOMTRUST))||
@@ -1479,22 +1482,22 @@
 
 		pdb_sethexpwd (temp, pdb_get_lanman_passwd(sampass),
 			       pdb_get_acct_ctrl(sampass));
-		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 			      PDB_LMPASSWD, "lmPassword", temp);
 
 		pdb_sethexpwd (temp, pdb_get_nt_passwd(sampass),
 			       pdb_get_acct_ctrl(sampass));
-		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 			      PDB_NTPASSWD, "ntPassword", temp);
 
 		slprintf (temp, sizeof (temp) - 1, "%li",
 			  pdb_get_pass_last_set_time(sampass));
-		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+		make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 			      PDB_PASSLASTSET, "pwdLastSet", temp);
 	}
 
 	/* FIXME: Hours stuff goes in LDAP  */
-	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, pdb_add,
+	make_ldap_mod(ldap_state->ldap_struct, existing, mods, sampass, need_update,
 		      PDB_ACCTCTRL, "acctFlags",
 		      pdb_encode_acct_ctrl (pdb_get_acct_ctrl(sampass),
 					    NEW_PW_FORMAT_SPACE_PADDED_LEN));
@@ -1941,6 +1944,16 @@
 }
 
 /**********************************************************************
+  Helper function to determine for update_sam_account whether
+  we need LDAP modification.
+*********************************************************************/
+static BOOL element_is_changed(const SAM_ACCOUNT *sampass,
+			       enum pdb_elements element)
+{
+	return IS_SAM_CHANGED(sampass, element);
+}
+
+/**********************************************************************
 Update SAM_ACCOUNT 
 *********************************************************************/
 static NTSTATUS ldapsam_update_sam_account(struct pdb_methods *my_methods, SAM_ACCOUNT * newpwd)
@@ -1967,7 +1980,8 @@
 	entry = ldap_first_entry(ldap_state->ldap_struct, result);
 	dn = ldap_get_dn(ldap_state->ldap_struct, entry);
 
-	if (!init_ldap_from_sam(ldap_state, entry, &mods, newpwd, False)) {
+	if (!init_ldap_from_sam(ldap_state, entry, &mods, newpwd,
+				element_is_changed)) {
 		DEBUG(0, ("ldapsam_update_sam_account: init_ldap_from_sam failed!\n"));
 		ldap_msgfree(result);
 		return NT_STATUS_UNSUCCESSFUL;
@@ -1997,6 +2011,17 @@
 }
 
 /**********************************************************************
+  Helper function to determine for update_sam_account whether
+  we need LDAP modification.
+*********************************************************************/
+static BOOL element_is_set_or_changed(const SAM_ACCOUNT *sampass,
+				      enum pdb_elements element)
+{
+	return (IS_SAM_SET(sampass, element) ||
+		IS_SAM_CHANGED(sampass, element));
+}
+
+/**********************************************************************
 Add SAM_ACCOUNT to LDAP 
 *********************************************************************/
 static NTSTATUS ldapsam_add_sam_account(struct pdb_methods *my_methods, SAM_ACCOUNT * newpwd)
@@ -2066,7 +2091,8 @@
                 }
 	}
 
-	if (!init_ldap_from_sam(ldap_state, entry, &mods, newpwd, True)) {
+	if (!init_ldap_from_sam(ldap_state, entry, &mods, newpwd,
+				element_is_set_or_changed)) {
 		DEBUG(0, ("ldapsam_add_sam_account: init_ldap_from_sam failed!\n"));
 		ldap_msgfree(result);
 		ldap_mods_free(mods, 1);


More information about the samba-technical mailing list