CodeSOD: Open And Shut

Our anonymous friend writes: I was tasked with figuring out why invalid XML was being output by a homegrown XML parser. As I looked into the code, I found the way this code handles writing out XML files"

Yes, it really does open and close the file handle for every xwrite call. This means that it opens and closes it 3 times PER TAG when writing out the XML.

// xml_t is - essentially - a linked list of xml nodes.void xmlwrite_file(xml_t* node, char* pFilename, int bNew){ char WriteBuff[1024 * 17]; char nodevalue[1024 * 16]; char* nodeattribs;//[ATTRIBUTE_LEN]; char nodename[256]; char spacer[16]; xml_t* pSib; int sc = 0; if (!node) { return; } nodeattribs = (char*)malloc(ATTRIBUTE_LEN); strcpy(nodename, ""); FillNodeName(nodename, node); strcpy(nodevalue, ""); FillNodeValue(nodevalue, node); strcpy(nodeattribs, ""); FillNodeAttribs(nodeattribs, node); if (strlen(nodeattribs)) { strcpy(spacer, " "); } else { strcpy(spacer, ""); } sprintf(WriteBuff, "pChild) { xwrite(pFilename, ">\n", 0); xmlwrite_file(node->pChild, pFilename, 0); sprintf(WriteBuff, "%s>\n", nodename); xwrite(pFilename, WriteBuff, 0); } else // no child { if (strlen(nodevalue)) { sprintf(WriteBuff, ">%s%s>\n", nodevalue, nodename); xwrite(pFilename, WriteBuff, 0); } else { xwrite(pFilename, " />\n", 0); } } pSib = node->pSibling; while (pSib) { xml_t* pHoldSib = pSib->pSibling; pSib->pSibling = NULL; xmlwrite_file(pSib, pFilename, bNew); pSib->pSibling = pHoldSib; pSib = pSib->pSibling; } fflush(stdout); free(nodeattribs);}void xwrite(char* pFileName, char* pData, int bNew){ if (strlen(pData) == 0) { return; } File* fOut; if (bNew) { fOut = fopen(pFileName, "w"); } else { fOut = fopen(pFilename, "a"); } if (fOut) { fwrite(pData, 1, strlen(pData), fOut); fflush(fOut); fclose(fOut); }}

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!