66cfabe5850f11b52956866d59e9c55ecaa799ce angie Thu Apr 24 15:03:57 2025 -0700 Fix a couple things found by valgrind. Using memmove instead of safecpy when ranges can overlap is safer, but using safecpy didn't seem to be causing bugs. However, failing to clip to length of codonNew when uppercasing caused memory corruption that caused eventual failure reported in #35577. valgrind rules. diff --git src/hg/cgilib/annoFormatVep.c src/hg/cgilib/annoFormatVep.c index 9941e60cc6c..4ee3c32ad9d 100644 --- src/hg/cgilib/annoFormatVep.c +++ src/hg/cgilib/annoFormatVep.c @@ -393,31 +393,31 @@ /* If seq is longer than an abbreviated version of itself, change it to the abbreviated version. */ { if (isEmpty(seq)) return; int len = strlen(seq); const int elipsLen = 3; char lengthNote[512]; safef(lengthNote, sizeof(lengthNote), "(%d %s)", len, unit); if (len > 2*baseCount + elipsLen + strlen(lengthNote)) { // First baseCount bases, then "...": int offset = baseCount; safecpy(seq+offset, len+1-offset, "..."); offset += elipsLen; // then last baseCount bases: - safecpy(seq+offset, len+1-offset, seq+len-baseCount); + memmove(seq+offset, seq+len-baseCount, baseCount); offset += baseCount; // then lengthNote: safecpy(seq+offset, len+1-offset, lengthNote); } } static void tweakStopCodonAndLimitLength(char *aaSeq, char *codonSeq) /* If aa from gpFx has a stop 'Z', replace it with "*". * If the strings are very long, truncate with a note about how long they are. */ { char *earlyStop = strchr(aaSeq, 'Z'); if (earlyStop) { earlyStop[0] = '*'; earlyStop[1] = '\0'; @@ -486,30 +486,39 @@ } afVepNextColumn(self->f, self->doHtml); fprintf(self->f, "%u", change->pepPosition+1); afVepNextColumn(self->f, self->doHtml); int variantFrame = change->cdsPosition % 3; strLower(change->codonOld); int upLen = min(strlen(change->codonOld+variantFrame), refLen); toUpperN(change->codonOld+variantFrame, upLen); strLower(change->codonNew); int alleleLength = altLen; // watch out for symbolic alleles [actually now that we're using txAlt we shouldn't see these]: if (sameString(change->txAlt, "") || sameString(change->txAlt, "<*>")) alleleLength = upLen; else if (startsWith("<", change->txAlt)) alleleLength = 0; + int codonNewLen = strlen(change->codonNew); + if (codonNewLen < variantFrame + alleleLength) + { + // This can happen when an insertion introduces a stop codon -- codonNew is shorter than + // we would otherwise expect. + alleleLength = codonNewLen - variantFrame; + if (alleleLength < 0) + alleleLength = 0; + } toUpperN(change->codonNew+variantFrame, alleleLength); tweakStopCodonAndLimitLength(change->aaOld, change->codonOld); tweakStopCodonAndLimitLength(change->aaNew, change->codonNew); fprintf(self->f, "%s/%s", dashForEmpty(change->aaOld), dashForEmpty(change->aaNew)); afVepNextColumn(self->f, self->doHtml); fprintf(self->f, "%s/%s", dashForEmpty(change->codonOld), dashForEmpty(change->codonNew)); afVepNextColumn(self->f, self->doHtml); } else if (gpFx->detailType == nonCodingExon) { struct nonCodingExon *change = &(gpFx->details.nonCodingExon); uint refLen = strlen(change->txRef), altLen = strlen(change->txAlt); boolean isInsertion = (refLen == 0); boolean isDeletion = altLen < refLen; int cDnaPosition = change->cDnaPosition;