From 5737b541866a42b1ef31285feb63db0a866299db Mon Sep 17 00:00:00 2001 From: rillig Date: Sat, 5 Dec 2020 15:35:34 +0000 Subject: [PATCH] make(1): indent targ.c with tabs instead of spaces Explain the tricky details of GNode_Free. Invert a condition in Targ_PrintNode to reduce the overall indentation. --- usr.bin/make/targ.c | 504 ++++++++++++++++++++++++-------------------- 1 file changed, 271 insertions(+), 233 deletions(-) diff --git a/usr.bin/make/targ.c b/usr.bin/make/targ.c index e732cffb7c22..5d564b85ada8 100644 --- a/usr.bin/make/targ.c +++ b/usr.bin/make/targ.c @@ -1,4 +1,4 @@ -/* $NetBSD: targ.c,v 1.149 2020/12/04 14:39:56 rillig Exp $ */ +/* $NetBSD: targ.c,v 1.150 2020/12/05 15:35:34 rillig Exp $ */ /* * Copyright (c) 1988, 1989, 1990, 1993 @@ -119,7 +119,7 @@ #include "dir.h" /* "@(#)targ.c 8.2 (Berkeley) 3/19/94" */ -MAKE_RCSID("$NetBSD: targ.c,v 1.149 2020/12/04 14:39:56 rillig Exp $"); +MAKE_RCSID("$NetBSD: targ.c,v 1.150 2020/12/05 15:35:34 rillig Exp $"); /* * All target nodes that appeared on the left-hand side of one of the @@ -137,24 +137,24 @@ static void GNode_Free(void *); void Targ_Init(void) { - HashTable_Init(&allTargetsByName); + HashTable_Init(&allTargetsByName); } void Targ_End(void) { - Targ_Stats(); + Targ_Stats(); #ifdef CLEANUP - Lst_Done(&allTargets); - HashTable_Done(&allTargetsByName); - Lst_DoneCall(&allNodes, GNode_Free); + Lst_Done(&allTargets); + HashTable_Done(&allTargetsByName); + Lst_DoneCall(&allNodes, GNode_Free); #endif } void Targ_Stats(void) { - HashTable_DebugStats(&allTargetsByName, "targets"); + HashTable_DebugStats(&allTargetsByName, "targets"); } /* @@ -165,10 +165,11 @@ Targ_Stats(void) GNodeList * Targ_List(void) { - return &allTargets; + return &allTargets; } -/* Create a new graph node, but don't register it anywhere. +/* + * Create a new graph node, but don't register it anywhere. * * Graph nodes that appear on the left-hand side of a dependency line such * as "target: source" are called targets. XXX: In some cases (like the @@ -185,68 +186,95 @@ Targ_List(void) GNode * GNode_New(const char *name) { - GNode *gn; + GNode *gn; - gn = bmake_malloc(sizeof *gn); - gn->name = bmake_strdup(name); - gn->uname = NULL; - gn->path = NULL; - gn->type = name[0] == '-' && name[1] == 'l' ? OP_LIB : 0; - gn->flags = 0; - gn->made = UNMADE; - gn->unmade = 0; - gn->mtime = 0; - gn->youngestChild = NULL; - Lst_Init(&gn->implicitParents); - Lst_Init(&gn->parents); - Lst_Init(&gn->children); - Lst_Init(&gn->order_pred); - Lst_Init(&gn->order_succ); - Lst_Init(&gn->cohorts); - gn->cohort_num[0] = '\0'; - gn->unmade_cohorts = 0; - gn->centurion = NULL; - gn->checked_seqno = 0; - HashTable_Init(&gn->vars); - Lst_Init(&gn->commands); - gn->suffix = NULL; - gn->fname = NULL; - gn->lineno = 0; + gn = bmake_malloc(sizeof *gn); + gn->name = bmake_strdup(name); + gn->uname = NULL; + gn->path = NULL; + gn->type = name[0] == '-' && name[1] == 'l' ? OP_LIB : 0; + gn->flags = 0; + gn->made = UNMADE; + gn->unmade = 0; + gn->mtime = 0; + gn->youngestChild = NULL; + Lst_Init(&gn->implicitParents); + Lst_Init(&gn->parents); + Lst_Init(&gn->children); + Lst_Init(&gn->order_pred); + Lst_Init(&gn->order_succ); + Lst_Init(&gn->cohorts); + gn->cohort_num[0] = '\0'; + gn->unmade_cohorts = 0; + gn->centurion = NULL; + gn->checked_seqno = 0; + HashTable_Init(&gn->vars); + Lst_Init(&gn->commands); + gn->suffix = NULL; + gn->fname = NULL; + gn->lineno = 0; #ifdef CLEANUP - Lst_Append(&allNodes, gn); + Lst_Append(&allNodes, gn); #endif - return gn; + return gn; } #ifdef CLEANUP static void GNode_Free(void *gnp) { - GNode *gn = gnp; + GNode *gn = gnp; - free(gn->name); - free(gn->uname); - free(gn->path); - /* gn->youngestChild is not owned by this node. */ - Lst_Done(&gn->implicitParents); /* Do not free the nodes themselves, */ - Lst_Done(&gn->parents); /* as they are not owned by this node. */ - Lst_Done(&gn->children); /* likewise */ - Lst_Done(&gn->order_pred); /* likewise */ - Lst_Done(&gn->order_succ); /* likewise */ - Lst_Done(&gn->cohorts); /* likewise */ - HashTable_Done(&gn->vars); /* Do not free the variables themselves, - * even though they are owned by this node. - * XXX: they should probably be freed. */ - Lst_Done(&gn->commands); /* Do not free the commands themselves, - * as they may be shared with other nodes. */ - /* gn->suffix is not owned by this node. */ - /* XXX: gn->suffix should be unreferenced here. This requires a thorough - * check that the reference counting is done correctly in all places, - * otherwise a suffix might be freed too early. */ + free(gn->name); + free(gn->uname); + free(gn->path); - free(gn); + /* Don't free gn->youngestChild since it is not owned by this node. */ + + /* + * In the following lists, only free the list nodes, but not the + * GNodes in them since these are not owned by this node. + */ + Lst_Done(&gn->implicitParents); + Lst_Done(&gn->parents); + Lst_Done(&gn->children); + Lst_Done(&gn->order_pred); + Lst_Done(&gn->order_succ); + Lst_Done(&gn->cohorts); + + /* + * Do not free the variables themselves, even though they are owned + * by this node. + * + * XXX: For the nodes that represent targets or sources (and not + * VAR_GLOBAL), it should be safe to free the variables as well, + * since each node manages the memory for all its variables itself. + * + * XXX: The GNodes that are only used as variable contexts (VAR_CMD, + * VAR_GLOBAL, VAR_INTERNAL) are not freed at all (see Var_End, where + * they are not mentioned). These might be freed at all, if their + * variable values are indeed not used anywhere else (see Trace_Init + * for the only suspicious use). + */ + HashTable_Done(&gn->vars); + + /* + * Do not free the commands themselves, as they may be shared with + * other nodes. + */ + Lst_Done(&gn->commands); + + /* + * gn->suffix is not owned by this node. + * + * XXX: gn->suffix should be unreferenced here. This requires a + * thorough check that the reference counting is done correctly in + * all places, otherwise a suffix might be freed too early. + */ + + free(gn); } #endif @@ -254,23 +282,23 @@ GNode_Free(void *gnp) GNode * Targ_FindNode(const char *name) { - return HashTable_FindValue(&allTargetsByName, name); + return HashTable_FindValue(&allTargetsByName, name); } /* Get the existing global node, or create it. */ GNode * Targ_GetNode(const char *name) { - Boolean isNew; - HashEntry *he = HashTable_CreateEntry(&allTargetsByName, name, &isNew); - if (!isNew) - return HashEntry_Get(he); + Boolean isNew; + HashEntry *he = HashTable_CreateEntry(&allTargetsByName, name, &isNew); + if (!isNew) + return HashEntry_Get(he); - { - GNode *gn = Targ_NewInternalNode(name); - HashEntry_Set(he, gn); - return gn; - } + { + GNode *gn = Targ_NewInternalNode(name); + HashEntry_Set(he, gn); + return gn; + } } /* @@ -282,13 +310,13 @@ Targ_GetNode(const char *name) GNode * Targ_NewInternalNode(const char *name) { - GNode *gn = GNode_New(name); - Var_Append(".ALLTARGETS", name, VAR_GLOBAL); - Lst_Append(&allTargets, gn); - DEBUG1(TARG, "Adding \"%s\" to all targets.\n", gn->name); - if (doing_depend) - gn->flags |= FROM_DEPEND; - return gn; + GNode *gn = GNode_New(name); + Var_Append(".ALLTARGETS", name, VAR_GLOBAL); + Lst_Append(&allTargets, gn); + DEBUG1(TARG, "Adding \"%s\" to all targets.\n", gn->name); + if (doing_depend) + gn->flags |= FROM_DEPEND; + return gn; } /* @@ -297,26 +325,30 @@ Targ_NewInternalNode(const char *name) */ GNode *Targ_GetEndNode(void) { - /* Save the node locally to avoid having to search for it all the time. */ - static GNode *endNode = NULL; - if (endNode == NULL) { - endNode = Targ_GetNode(".END"); - endNode->type = OP_SPECIAL; - } - return endNode; + /* + * Save the node locally to avoid having to search for it all + * the time. + */ + static GNode *endNode = NULL; + + if (endNode == NULL) { + endNode = Targ_GetNode(".END"); + endNode->type = OP_SPECIAL; + } + return endNode; } /* Add the named nodes to the list, creating them as necessary. */ void Targ_FindList(GNodeList *gns, StringList *names) { - StringListNode *ln; + StringListNode *ln; - for (ln = names->first; ln != NULL; ln = ln->next) { - const char *name = ln->datum; - GNode *gn = Targ_GetNode(name); - Lst_Append(gns, gn); - } + for (ln = names->first; ln != NULL; ln = ln->next) { + const char *name = ln->datum; + GNode *gn = Targ_GetNode(name); + Lst_Append(gns, gn); + } } /* @@ -326,22 +358,22 @@ Targ_FindList(GNodeList *gns, StringList *names) Boolean Targ_Ignore(const GNode *gn) { - return opts.ignoreErrors || gn->type & OP_IGNORE; + return opts.ignoreErrors || gn->type & OP_IGNORE; } /* Return true if be silent when creating gn. */ Boolean Targ_Silent(const GNode *gn) { - return opts.beSilent || gn->type & OP_SILENT; + return opts.beSilent || gn->type & OP_SILENT; } /* See if the given target is precious. */ Boolean Targ_Precious(const GNode *gn) { - /* XXX: Why are '::' targets precious? */ - return allPrecious || gn->type & (OP_PRECIOUS | OP_DOUBLEDEP); + /* XXX: Why are '::' targets precious? */ + return allPrecious || gn->type & (OP_PRECIOUS | OP_DOUBLEDEP); } /* @@ -354,158 +386,162 @@ static GNode *mainTarg; void Targ_SetMain(GNode *gn) { - mainTarg = gn; + mainTarg = gn; } static void PrintNodeNames(GNodeList *gnodes) { - GNodeListNode *ln; + GNodeListNode *ln; - for (ln = gnodes->first; ln != NULL; ln = ln->next) { - GNode *gn = ln->datum; - debug_printf(" %s%s", gn->name, gn->cohort_num); - } + for (ln = gnodes->first; ln != NULL; ln = ln->next) { + GNode *gn = ln->datum; + debug_printf(" %s%s", gn->name, gn->cohort_num); + } } static void PrintNodeNamesLine(const char *label, GNodeList *gnodes) { - if (Lst_IsEmpty(gnodes)) - return; - debug_printf("# %s:", label); - PrintNodeNames(gnodes); - debug_printf("\n"); + if (Lst_IsEmpty(gnodes)) + return; + debug_printf("# %s:", label); + PrintNodeNames(gnodes); + debug_printf("\n"); } void Targ_PrintCmds(GNode *gn) { - StringListNode *ln; - for (ln = gn->commands.first; ln != NULL; ln = ln->next) { - const char *cmd = ln->datum; - debug_printf("\t%s\n", cmd); - } + StringListNode *ln; + + for (ln = gn->commands.first; ln != NULL; ln = ln->next) { + const char *cmd = ln->datum; + debug_printf("\t%s\n", cmd); + } } -/* Format a modification time in some reasonable way and return it. - * The time is placed in a static area, so it is overwritten with each call. */ +/* + * Format a modification time in some reasonable way and return it. + * The time is placed in a static area, so it is overwritten with each call. + */ char * Targ_FmtTime(time_t tm) { - struct tm *parts; - static char buf[128]; + struct tm *parts; + static char buf[128]; - /* TODO: Add special case for 0, which often means ENOENT, to make it - * independent from time zones. */ - parts = localtime(&tm); - (void)strftime(buf, sizeof buf, "%k:%M:%S %b %d, %Y", parts); - return buf; + /* TODO: Add special case for 0, which often means ENOENT, to make it + * independent from time zones. */ + parts = localtime(&tm); + (void)strftime(buf, sizeof buf, "%k:%M:%S %b %d, %Y", parts); + return buf; } /* Print out a type field giving only those attributes the user can set. */ void Targ_PrintType(int type) { - int tbit; + int tbit; -#define PRINTBIT(attr) case CONCAT(OP_,attr): debug_printf(" ." #attr); break +#define PRINTBIT(attr) case CONCAT(OP_,attr): debug_printf(" ." #attr); break #define PRINTDBIT(attr) case CONCAT(OP_,attr): if (DEBUG(TARG))debug_printf(" ." #attr); break - type &= ~OP_OPMASK; + type &= ~OP_OPMASK; - while (type != 0) { - tbit = 1 << (ffs(type) - 1); - type &= ~tbit; + while (type != 0) { + tbit = 1 << (ffs(type) - 1); + type &= ~tbit; - switch(tbit) { - PRINTBIT(OPTIONAL); - PRINTBIT(USE); - PRINTBIT(EXEC); - PRINTBIT(IGNORE); - PRINTBIT(PRECIOUS); - PRINTBIT(SILENT); - PRINTBIT(MAKE); - PRINTBIT(JOIN); - PRINTBIT(INVISIBLE); - PRINTBIT(NOTMAIN); - PRINTDBIT(LIB); - /*XXX: MEMBER is defined, so CONCAT(OP_,MEMBER) gives OP_"%" */ - case OP_MEMBER: if (DEBUG(TARG))debug_printf(" .MEMBER"); break; - PRINTDBIT(ARCHV); - PRINTDBIT(MADE); - PRINTDBIT(PHONY); + switch (tbit) { + PRINTBIT(OPTIONAL); + PRINTBIT(USE); + PRINTBIT(EXEC); + PRINTBIT(IGNORE); + PRINTBIT(PRECIOUS); + PRINTBIT(SILENT); + PRINTBIT(MAKE); + PRINTBIT(JOIN); + PRINTBIT(INVISIBLE); + PRINTBIT(NOTMAIN); + PRINTDBIT(LIB); + /*XXX: MEMBER is defined, so CONCAT(OP_,MEMBER) gives OP_"%" */ + case OP_MEMBER: if (DEBUG(TARG))debug_printf(" .MEMBER"); break; + PRINTDBIT(ARCHV); + PRINTDBIT(MADE); + PRINTDBIT(PHONY); + } } - } } static const char * made_name(GNodeMade made) { - switch (made) { - case UNMADE: return "unmade"; - case DEFERRED: return "deferred"; - case REQUESTED: return "requested"; - case BEINGMADE: return "being made"; - case MADE: return "made"; - case UPTODATE: return "up-to-date"; - case ERROR: return "error when made"; - case ABORTED: return "aborted"; - default: return "unknown enum_made value"; - } + switch (made) { + case UNMADE: return "unmade"; + case DEFERRED: return "deferred"; + case REQUESTED: return "requested"; + case BEINGMADE: return "being made"; + case MADE: return "made"; + case UPTODATE: return "up-to-date"; + case ERROR: return "error when made"; + case ABORTED: return "aborted"; + default: return "unknown enum_made value"; + } } static const char * GNode_OpName(const GNode *gn) { - switch (gn->type & OP_OPMASK) { - case OP_DEPENDS: - return ":"; - case OP_FORCE: - return "!"; - case OP_DOUBLEDEP: - return "::"; - } - return ""; + switch (gn->type & OP_OPMASK) { + case OP_DEPENDS: + return ":"; + case OP_FORCE: + return "!"; + case OP_DOUBLEDEP: + return "::"; + } + return ""; } /* Print the contents of a node. */ void Targ_PrintNode(GNode *gn, int pass) { - debug_printf("# %s%s", gn->name, gn->cohort_num); - GNode_FprintDetails(opts.debug_file, ", ", gn, "\n"); - if (gn->flags == 0) - return; + debug_printf("# %s%s", gn->name, gn->cohort_num); + GNode_FprintDetails(opts.debug_file, ", ", gn, "\n"); + if (gn->flags == 0) + return; + + if (!GNode_IsTarget(gn)) + return; - if (GNode_IsTarget(gn)) { debug_printf("#\n"); - if (gn == mainTarg) { - debug_printf("# *** MAIN TARGET ***\n"); - } + if (gn == mainTarg) + debug_printf("# *** MAIN TARGET ***\n"); + if (pass >= 2) { - if (gn->unmade > 0) { - debug_printf("# %d unmade children\n", gn->unmade); - } else { - debug_printf("# No unmade children\n"); - } - if (!(gn->type & (OP_JOIN|OP_USE|OP_USEBEFORE|OP_EXEC))) { - if (gn->mtime != 0) { - debug_printf("# last modified %s: %s\n", - Targ_FmtTime(gn->mtime), - made_name(gn->made)); - } else if (gn->made != UNMADE) { - debug_printf("# non-existent (maybe): %s\n", - made_name(gn->made)); - } else { - debug_printf("# unmade\n"); + if (gn->unmade > 0) + debug_printf("# %d unmade children\n", gn->unmade); + else + debug_printf("# No unmade children\n"); + if (!(gn->type & (OP_JOIN | OP_USE | OP_USEBEFORE | OP_EXEC))) { + if (gn->mtime != 0) { + debug_printf("# last modified %s: %s\n", + Targ_FmtTime(gn->mtime), + made_name(gn->made)); + } else if (gn->made != UNMADE) { + debug_printf("# non-existent (maybe): %s\n", + made_name(gn->made)); + } else + debug_printf("# unmade\n"); } - } - PrintNodeNamesLine("implicit parents", &gn->implicitParents); + PrintNodeNamesLine("implicit parents", &gn->implicitParents); } else { - if (gn->unmade) - debug_printf("# %d unmade children\n", gn->unmade); + if (gn->unmade) + debug_printf("# %d unmade children\n", gn->unmade); } + PrintNodeNamesLine("parents", &gn->parents); PrintNodeNamesLine("order_pred", &gn->order_pred); PrintNodeNamesLine("order_succ", &gn->order_succ); @@ -516,38 +552,38 @@ Targ_PrintNode(GNode *gn, int pass) debug_printf("\n"); Targ_PrintCmds(gn); debug_printf("\n\n"); - if (gn->type & OP_DOUBLEDEP) { - Targ_PrintNodes(&gn->cohorts, pass); - } - } + if (gn->type & OP_DOUBLEDEP) + Targ_PrintNodes(&gn->cohorts, pass); } void Targ_PrintNodes(GNodeList *gnodes, int pass) { - GNodeListNode *ln; - for (ln = gnodes->first; ln != NULL; ln = ln->next) - Targ_PrintNode(ln->datum, pass); + GNodeListNode *ln; + + for (ln = gnodes->first; ln != NULL; ln = ln->next) + Targ_PrintNode(ln->datum, pass); } /* Print only those targets that are just a source. */ static void PrintOnlySources(void) { - GNodeListNode *ln; + GNodeListNode *ln; - for (ln = allTargets.first; ln != NULL; ln = ln->next) { - GNode *gn = ln->datum; - if (GNode_IsTarget(gn)) - continue; + for (ln = allTargets.first; ln != NULL; ln = ln->next) { + GNode *gn = ln->datum; + if (GNode_IsTarget(gn)) + continue; - debug_printf("#\t%s [%s]", gn->name, GNode_Path(gn)); - Targ_PrintType(gn->type); - debug_printf("\n"); - } + debug_printf("#\t%s [%s]", gn->name, GNode_Path(gn)); + Targ_PrintType(gn->type); + debug_printf("\n"); + } } -/* Input: +/* + * Input: * pass 1 => before processing * 2 => after processing * 3 => after processing, an error occurred @@ -555,49 +591,51 @@ PrintOnlySources(void) void Targ_PrintGraph(int pass) { - debug_printf("#*** Input graph:\n"); - Targ_PrintNodes(&allTargets, pass); - debug_printf("\n"); - debug_printf("\n"); + debug_printf("#*** Input graph:\n"); + Targ_PrintNodes(&allTargets, pass); + debug_printf("\n"); + debug_printf("\n"); - debug_printf("#\n"); - debug_printf("# Files that are only sources:\n"); - PrintOnlySources(); + debug_printf("#\n"); + debug_printf("# Files that are only sources:\n"); + PrintOnlySources(); - debug_printf("#*** Global Variables:\n"); - Var_Dump(VAR_GLOBAL); + debug_printf("#*** Global Variables:\n"); + Var_Dump(VAR_GLOBAL); - debug_printf("#*** Command-line Variables:\n"); - Var_Dump(VAR_CMDLINE); + debug_printf("#*** Command-line Variables:\n"); + Var_Dump(VAR_CMDLINE); - debug_printf("\n"); - Dir_PrintDirectories(); - debug_printf("\n"); + debug_printf("\n"); + Dir_PrintDirectories(); + debug_printf("\n"); - Suff_PrintAll(); + Suff_PrintAll(); } -/* Propagate some type information to cohort nodes (those from the '::' +/* + * Propagate some type information to cohort nodes (those from the '::' * dependency operator). * * Should be called after the makefiles are parsed but before any action is - * taken. */ + * taken. + */ void Targ_Propagate(void) { - GNodeListNode *ln, *cln; + GNodeListNode *ln, *cln; - for (ln = allTargets.first; ln != NULL; ln = ln->next) { - GNode *gn = ln->datum; - GNodeType type = gn->type; + for (ln = allTargets.first; ln != NULL; ln = ln->next) { + GNode *gn = ln->datum; + GNodeType type = gn->type; - if (!(type & OP_DOUBLEDEP)) - continue; + if (!(type & OP_DOUBLEDEP)) + continue; - for (cln = gn->cohorts.first; cln != NULL; cln = cln->next) { - GNode *cohort = cln->datum; + for (cln = gn->cohorts.first; cln != NULL; cln = cln->next) { + GNode *cohort = cln->datum; - cohort->type |= type & ~OP_OPMASK; + cohort->type |= type & ~OP_OPMASK; + } } - } }