Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update to fail on duplicate LN tags, with different LN values #1882

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion header.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static int sam_hrecs_update_hashes(sam_hrecs_t *hrecs,
const char *name = NULL;
const char *altnames = NULL;
hts_pos_t len = -1;
int r;
int r, dup = 0;
khint_t k;

while (tag) {
Expand All @@ -154,7 +154,12 @@ static int sam_hrecs_update_hashes(sam_hrecs_t *hrecs,
name = tag->str+3;
} else if (tag->str[0] == 'L' && tag->str[1] == 'N') {
assert(tag->len >= 3);
hts_pos_t tmp = len;
len = strtoll(tag->str+3, NULL, 10);
if (tmp != -1 && tmp != len) {
//LN again with different value, discard
dup = 1;
}
} else if (tag->str[0] == 'A' && tag->str[1] == 'N') {
assert(tag->len >= 3);
altnames = tag->str+3;
Expand All @@ -173,6 +178,12 @@ static int sam_hrecs_update_hashes(sam_hrecs_t *hrecs,
return -1; // LN should be present, according to spec.
}

if (dup) {
hts_log_error("Header includes @SQ line \"%s\" with duplicate LN: tag", \
name);
return -1;
}

// Seen already?
k = kh_get(m_s2i, hrecs->ref_hash, name);
if (k < kh_end(hrecs->ref_hash)) {
Expand Down
18 changes: 16 additions & 2 deletions sam.c
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,12 @@ static sam_hdr_t *sam_hdr_sanitise(sam_hdr_t *h) {
cp[h->l_text] = '\0';
}

if (sam_hdr_fill_hrecs(h) < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want to call sam_hdr_fill_hrecs() here. It will trigger a full parse of the header, which we may not want to do unless really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

//failed in parsing / validation
sam_hdr_destroy(h);
return NULL;
}

return h;
}

Expand Down Expand Up @@ -1964,8 +1970,16 @@ static sam_hdr_t *sam_hdr_create(htsFile* fp) {
strncpy(sn, q, r - q);
q = r;
} else {
if (strncmp(q, "LN:", 3) == 0)
ln = strtoll(q + 3, (char**)&q, 10);
if (strncmp(q, "LN:", 3) == 0) {
hts_pos_t tmp = strtoll(q + 3, (char**)&q, 10);
if (ln != -1 && tmp != ln) {
//duplicate LN tag with different value
hts_log_error("111Header includes @SQ line \"%s\" with \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message looks a bit odd...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

duplicate LN: tag", sn);
goto error;
}
ln = tmp;
}
}

while (*q != '\t' && *q != '\n' && *q != '\0')
Expand Down