Сколько людей пользуются субтитрами по всему миру? Вероятно, очень много. В образовательных целях или просто из-за любви к оригинальной озвучке, в интернете можно найти субтитры практически к любому фильму и на многих языках. Создаётся всё это в специальных программах. Как и в большинстве программ, в Subtitle Edit не обошлось без сюрпризов в виде багов.
Subtitle Edit - бесплатный редактор с огромным списком возможностей. Это большой проект на языке C# с открытым исходным кодом. Программа очень популярна и выдаётся в первых строках выдачи результатов поисковиков, на сайте проекта перечислено множество наград. В репозитории на GitHub можно увидеть, что проект активно развивается, имеет много звёзд и форков. В общем, это хороший проект, чтобы поучаствовать в его развитии. Изначально я просто искал себе библиотеку для парсинга субтитров, потому что большинство форматов не текстовые, но теперь вернусь к своему проекту немного позже.
На странице проекта на GitHub открыто 310 Issues. Возможно, работа с результатами анализа позволит что-то исправить. Статический анализатор PVS-Studio, который использовался для исследования кода, выдал 460 предупреждений (суммарно для всех уровней важности). Практически всё можно и нужно поправить. Это связано с тем, что в анализаторе почти нет рекомендательных диагностик. Найденные результаты обычно сигнализируют о реальных проблемах в коде. В статье я буду приводить примеры кода, но выберу только те ошибки, которые могут сильно влиять на работу.
Для более-менее понятных фрагментов кода я отправлю Pull Request с исправлениями. Но автору проекта лучше ознакомиться со всеми результатами анализа, проверив проект самостоятельно.
Так выглядит фрагмент формы для задания стиля субтитров:
А вот предупреждение анализатора на код, который связан с этой формой:
V3003 CWE-570 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 300, 302. SubStationAlphaStyles.cs 300
public static void AddStyle(ListView lv, SsaStyle ssaStyle,
Subtitle subtitle, bool isSubstationAlpha)
{
....
if (ssaStyle.Bold || ssaStyle.Italic)
subItem.Font = new Font(...., FontStyle.Bold |
FontStyle.Italic);
else if (ssaStyle.Bold)
subItem.Font = new Font(...., FontStyle.Bold);
else if (ssaStyle.Italic)
subItem.Font = new Font(...., FontStyle.Italic);
else if (ssaStyle.Italic)
subItem.Font = new Font(...., FontStyle.Regular);
....
}Всего анализатор выдал 4 предупреждения на этот фрагмент кода. И это не удивительно, ведь тут ошибка почти в каждой строке. Более того, тут не рассмотрен вариант с ssaStyle.Underline!
Код лучше переписать на что-то подобное и сделать это очень внимательно:
....
if (ssaStyle.Bold)
fontStyles |= FontStyle.Bold;
....
subItem.Font = new Font(...., fontStyles);
....V3022 CWE-570 Expression '_networkSession != null && _networkSession.LastSubtitle != null && i < _networkSession.LastSubtitle.Paragraphs.Count' is always false. Main.cs 7242
private void DeleteSelectedLines()
{
....
if (_networkSession != null) // <=
{
_networkSession.TimerStop();
NetworkGetSendUpdates(indices, 0, null);
}
else
{
indices.Reverse();
foreach (int i in indices)
{
_subtitle.Paragraphs.RemoveAt(i);
if (_networkSession != null && // <=
_networkSession.LastSubtitle != null &&
i < _networkSession.LastSubtitle.Paragraphs.Count)
_networkSession.LastSubtitle.Paragraphs.RemoveAt(i);
}
....
}
....
}Переменная _networkSession уже проверена в первом условии, следовательно, в ветви else она гарантированно будет иметь значение null. Такое сочетание проверок привело к ложному условию и недостижимому коду.
V3003 CWE-570 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 113, 115. SsaStyle.cs 113
public string ToRawSsa(string styleFormat)
{
var sb = new StringBuilder();
sb.Append("Style: ");
var format = ....;
for (int i = 0; i < format.Length; i++)
{
string f = format[i].Trim();
if (f == "name")
sb.Append(Name);
....
else if (f == "shadow") // <=
sb.Append(OutlineWidth); // <=
else if (f == "shadow") // <=
sb.Append(ShadowWidth); // <=
....
}
....
}Опечатки в каскаде условий приводят к появлению недостижимых ветвей кода. Часто такой код является следствием Copy-Paste программирования. В приведённом примере второе продублированное условие никогда не будет выполнено. И это самый простой и компактный пример, который я выбрал для статьи. Таких примеров в проекте было найдено достаточно много, чтобы описать проблему в отдельном разделе.
Вот весь список Copy-Paste кода, требующего исправления:
V3022 CWE-570 Expression 'param.Bitmap.Width == 720 && param.Bitmap.Width == 480' is always false. Probably the '||' operator should be used here. ExportPngXml.cs 1808
private static string FormatFabTime(TimeCode time,
MakeBitmapParameter param)
{
if (param.Bitmap.Width == 720 && param.Bitmap.Width == 480)
return $"....";
// drop frame
if (Math.Abs(param.... - 24 * (999 / 1000)) < 0.01 ||
Math.Abs(param.... - 29 * (999 / 1000)) < 0.01 ||
Math.Abs(param.... - 59 * (999 / 1000)) < 0.01)
return $"....";
return $"....";
}Путаница с Width и Height является классическим примером опечатки. Но в этой функции подозрительно ещё кое-что. Все сокращения строки, которые я заменил четырьмя многоточиями, это одна и та же строка: {time.Hours:00};{time.Minutes:00};{time.Seconds:00};{SubtitleFormat.MillisecondsToFramesMaxFrameRate(time.Milliseconds):00}. Т.е. наличие двух условий никак не влияет на результат функции, функция всегда возвращает одно и то же.
V3009 CWE-393 It's odd that this method always returns one and the same value of 'true'. Main.cs 10153
private bool LoadTextSTFromMatroska(
MatroskaTrackInfo matroskaSubtitleInfo,
MatroskaFile matroska,
bool batchMode)
{
....
_fileDateTime = new DateTime();
_converted = true;
if (batchMode)
return true;
SubtitleListview1.Fill(_subtitle, _subtitleAlternate);
if (_subtitle.Paragraphs.Count > 0)
SubtitleListview1.SelectIndexAndEnsureVisible(0);
ShowSource();
return true;
}Найдена функция, которая всегда возвращает значение true. Вероятно, это ошибка. Значение этой функции проверяется в четырёх местах программы. Также рядом в коде присутствуют похожие функции, например, LoadDvbFromMatroska(), и она возвращает разные значения.
V3022 CWE-571 Expression 'listBoxVobFiles.Items.Count > 0' is always true. DvdSubRip.cs 533
private void DvdSubRip_Shown(object sender, EventArgs e)
{
if (string.IsNullOrEmpty(_initialFileName))
return;
if (_initialFileName.EndsWith(".ifo", ....))
{
OpenIfoFile(_initialFileName);
}
else if (_initialFileName.EndsWith(".vob", ....))
{
listBoxVobFiles.Items.Add(_initialFileName);
buttonStartRipping.Enabled = listBoxVobFiles.Items.Count > 0;
}
_initialFileName = null;
}В список listBoxVobFiles добавляется элемент, а потом проверяется, не пустой ли список. Очевидно, что там будет как минимум один элемент. И подобных проверок, которые всегда истинны или ложны, в проекте более тридцати.
V3005 The 'positionInfo' variable is assigned to itself. WebVTT.cs 79
internal static string GetPositionInfoFromAssTag(Paragraph p)
{
....
if (!string.IsNullOrEmpty(line))
{
if (positionInfo == null)
positionInfo = " line:" + line;
else
positionInfo = positionInfo += " line:" + line;
}
....
}Выбирая между вариантами записи "A = A + n" и "A += n", автор этого кода выбрал компромиссный вариант "A = A += n" :D
Для понимания, как исправить то или иное предупреждение анализатора, нужно немного разбираться в форматах субтитров и особенностях их обработки. Поэтому если есть желающие поддержать проект и покидать Pull Request'ы с исправлениями автору проекта на GitHub, то вот ссылка для загрузки HTML отчёта с предупреждениями PVS-Studio уровня High/Medium.